You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "yuzelin (via GitHub)" <gi...@apache.org> on 2023/05/23 13:18:00 UTC
[GitHub] [incubator-paimon] yuzelin opened a new pull request, #1213: [hive] Hive read paimon table should always build schema from existing Paimon table schema first
yuzelin opened a new pull request, #1213:
URL: https://github.com/apache/incubator-paimon/pull/1213
### Purpose
When reading from Hive, the current `HiveSchema` maybe constructed from Hive schema infomation even Paimon schema is available. This PR fix it.
This PR can fix #1198 because now the `TIMESTAMP` precision can be set correctly.
### Tests
`PaimonStorageHandlerITCase#testDateAndTimestamp`
### API and Format
no
### Documentation
no
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1213: [hive] Hive read paimon table should always build schema from existing Paimon table schema first
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1213:
URL: https://github.com/apache/incubator-paimon/pull/1213#discussion_r1202374921
##########
paimon-hive/paimon-hive-connector-common/src/test/java/org/apache/paimon/hive/PaimonStorageHandlerITCase.java:
##########
@@ -900,14 +900,25 @@ public void testPredicatePushDown() throws Exception {
public void testDateAndTimestamp() throws Exception {
String path = folder.newFolder().toURI().toString();
String tablePath = String.format("%s/default.db/hive_test_table", path);
+ ThreadLocalRandom random = ThreadLocalRandom.current();
Options conf = new Options();
conf.set(CatalogOptions.WAREHOUSE, path);
- conf.set(CoreOptions.FILE_FORMAT, CoreOptions.FileFormatType.AVRO);
+ int randomInt = random.nextInt(3);
+ if (randomInt == 0) {
+ conf.set(CoreOptions.FILE_FORMAT, CoreOptions.FileFormatType.ORC);
+ } else if (randomInt == 1) {
+ conf.set(CoreOptions.FILE_FORMAT, CoreOptions.FileFormatType.PARQUET);
+ } else {
+ conf.set(CoreOptions.FILE_FORMAT, CoreOptions.FileFormatType.AVRO);
+ }
Table table =
FileStoreTestUtils.createFileStoreTable(
conf,
RowType.of(
- new DataType[] {DataTypes.DATE(), DataTypes.TIMESTAMP(3)},
+ new DataType[] {
+ DataTypes.DATE(),
+ DataTypes.TIMESTAMP(random.nextBoolean() ? 3 : 6)
Review Comment:
random.nextInt(10)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1213: [hive] Hive read paimon table should always build schema from existing Paimon table schema first
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1213:
URL: https://github.com/apache/incubator-paimon/pull/1213#discussion_r1202378367
##########
paimon-hive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/HiveSchema.java:
##########
@@ -112,21 +111,24 @@ public static HiveSchema extract(@Nullable Configuration configuration, Properti
List<String> columnNames = Arrays.asList(columnProperty.split(columnNameDelimiter));
String columnTypes = properties.getProperty(serdeConstants.LIST_COLUMN_TYPES);
List<TypeInfo> typeInfos = TypeInfoUtils.getTypeInfosFromTypeString(columnTypes);
+
+ // If both Paimon table schema and Hive table schema exist, we check whether the schema
+ // matches and still build from Paimon table schema
+ if (tableSchema.isPresent()) {
+ if (columnNames.size() > 0 && typeInfos.size() > 0) {
+ LOG.debug(
+ "Extract schema with exists DDL and exists paimon table, table location:[{}].",
+ location);
+ checkSchemaMatched(columnNames, typeInfos, tableSchema.get());
+ }
+
+ return new HiveSchema(new RowType(tableSchema.get().fields()));
Review Comment:
Don't just return this.
For example, Hive DDL can be f1, f2, but table can be f0,f1,f2,f3,f4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1213: [hive] Hive read paimon table should always build schema from existing Paimon table schema first
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1213:
URL: https://github.com/apache/incubator-paimon/pull/1213#discussion_r1202378367
##########
paimon-hive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/HiveSchema.java:
##########
@@ -112,21 +111,24 @@ public static HiveSchema extract(@Nullable Configuration configuration, Properti
List<String> columnNames = Arrays.asList(columnProperty.split(columnNameDelimiter));
String columnTypes = properties.getProperty(serdeConstants.LIST_COLUMN_TYPES);
List<TypeInfo> typeInfos = TypeInfoUtils.getTypeInfosFromTypeString(columnTypes);
+
+ // If both Paimon table schema and Hive table schema exist, we check whether the schema
+ // matches and still build from Paimon table schema
+ if (tableSchema.isPresent()) {
+ if (columnNames.size() > 0 && typeInfos.size() > 0) {
+ LOG.debug(
+ "Extract schema with exists DDL and exists paimon table, table location:[{}].",
+ location);
+ checkSchemaMatched(columnNames, typeInfos, tableSchema.get());
+ }
+
+ return new HiveSchema(new RowType(tableSchema.get().fields()));
Review Comment:
Don't do this.
For example, Hive DDL can be f1, f2, but table can be f0,f1,f2,f3,f4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] JingsongLi merged pull request #1213: [hive] Hive read paimon table should always build schema from existing Paimon table schema first
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #1213:
URL: https://github.com/apache/incubator-paimon/pull/1213
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] yuzelin commented on a diff in pull request #1213: [hive] Hive read paimon table should always build schema from existing Paimon table schema first
Posted by "yuzelin (via GitHub)" <gi...@apache.org>.
yuzelin commented on code in PR #1213:
URL: https://github.com/apache/incubator-paimon/pull/1213#discussion_r1203344135
##########
paimon-hive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/HiveSchema.java:
##########
@@ -112,21 +111,24 @@ public static HiveSchema extract(@Nullable Configuration configuration, Properti
List<String> columnNames = Arrays.asList(columnProperty.split(columnNameDelimiter));
String columnTypes = properties.getProperty(serdeConstants.LIST_COLUMN_TYPES);
List<TypeInfo> typeInfos = TypeInfoUtils.getTypeInfosFromTypeString(columnTypes);
+
+ // If both Paimon table schema and Hive table schema exist, we check whether the schema
+ // matches and still build from Paimon table schema
+ if (tableSchema.isPresent()) {
+ if (columnNames.size() > 0 && typeInfos.size() > 0) {
+ LOG.debug(
+ "Extract schema with exists DDL and exists paimon table, table location:[{}].",
+ location);
+ checkSchemaMatched(columnNames, typeInfos, tableSchema.get());
+ }
+
+ return new HiveSchema(new RowType(tableSchema.get().fields()));
Review Comment:
modified.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1213: [hive] Hive read paimon table should always build schema from existing Paimon table schema first
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1213:
URL: https://github.com/apache/incubator-paimon/pull/1213#discussion_r1203375084
##########
paimon-hive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/HiveSchema.java:
##########
@@ -121,16 +125,20 @@ public static HiveSchema extract(@Nullable Configuration configuration, Properti
"Extract schema with exists DDL and exists paimon table, table location:[{}].",
location);
checkSchemaMatched(columnNames, typeInfos, tableSchema.get());
- // Use paimon table column comment when the paimon table exists.
- comments =
+ // Use paimon table data types and column comments when the paimon table exists.
Review Comment:
why? add comment?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org