wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16512 )
Change subject: IMPALA-10187: Add PARTITON BY SPEC to SHOW CREATE TABLE for Iceberg Tables ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Hi Gabor, thanks for applying the changes, LGTM! And I will give you a review+1. But I'm not a committer, I'm not sure my reivew is valid for your patch. http://gerrit.cloudera.org:8080/#/c/16512/1/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionField.java File fe/src/main/java/org/apache/impala/analysis/IcebergPartitionField.java: http://gerrit.cloudera.org:8080/#/c/16512/1/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionField.java@79 PS1, Line 79: // Iceberg library's PartitionSpec.Builder adds a postfix to the field name that : // represents the transform type of that field. E.g. "partition_name Day" would be : // named partition_name_day. This function returns the field name without that postfix. : private String fieldNameWithoutPostfix() { : String transformPostfix = "_" + fieldType_.toString(); : if (fieldName_.toUpperCase().endsWith(transformPostfix)) { : return fieldName_.substring(0, fieldName_.length() - transformPostfix.length()); : } : return fieldName_; : } > I'm not sure I understand the comment, but it's not enough to save the trun I mean can we add a new field such as 'columnName_' to record source column name in IcebergPartitionField class. And this value is obtain from Iceberg directly, instead of truncate from 'fieldName_'. Maybe we can use 'sourceId' to get source column name by TableMetadata when construct IcebergPartitionField. In this case, even if Iceberg add other partition field naming rules, we can also reuse code. Maybe we can consider this in a new patch. fieldNameWithoutPostfix() is enough for this patch. -- To view, visit http://gerrit.cloudera.org:8080/16512 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie4c43b75057807ab513a220d348155be2487e714 Gerrit-Change-Number: 16512 Gerrit-PatchSet: 2 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: wangsheng <[email protected]> Gerrit-Comment-Date: Wed, 30 Sep 2020 03:03:53 +0000 Gerrit-HasComments: Yes
