Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17575 )

Change subject: IMPALA-10732: Use consistent DDL for specifying Iceberg 
partitions
......................................................................


Patch Set 4:

(4 comments)

Thanks for the comments!

http://gerrit.cloudera.org:8080/#/c/17575/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17575/4//COMMIT_MSG@33
PS4, Line 33: makes Impala to use
> typo: makes Impala use
Done


http://gerrit.cloudera.org:8080/#/c/17575/4/fe/src/main/java/org/apache/impala/util/IcebergUtil.java
File fe/src/main/java/org/apache/impala/util/IcebergUtil.java:

http://gerrit.cloudera.org:8080/#/c/17575/4/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@291
PS4, Line 291: transformType.startsWit
> Not your change, but why is startsWith() used instead of equals() for BUCKE
Because transformType might contain the parameters as well (e.g. "BUCKET[5]") 
when we process the partition spec loaded from iceberg.


http://gerrit.cloudera.org:8080/#/c/17575/4/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@302
PS4, Line 302: "Unsupported iceberg partition type: "
> Do we have a test that exercises this error message?
Added a test case to iceberg-negative.test


http://gerrit.cloudera.org:8080/#/c/17575/4/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@296
PS4, Line 296:   switch (transformType) {
             :       case "HOUR":  case "HOURS":  return 
TIcebergPartitionTransformType.HOUR;
             :       case "DAY":   case "DAYS":   return 
TIcebergPartitionTransformType.DAY;
             :       case "MONTH": case "MONTHS": return 
TIcebergPartitionTransformType.MONTH;
             :       case "YEAR":  case "YEARS":  return 
TIcebergPartitionTransformType.YEAR;
             :       default:
             :         throw new TableLoadingException("Unsupported iceberg 
partition type: " +
             :             transformType);
             :     }
> nit: Maybe adding these transform type strings and the ones above to a Stri
'transformType' for BUCKET and TRUNCATE might contain the parameters as well, 
see above.



--
To view, visit http://gerrit.cloudera.org:8080/17575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib72ae445fd68fb0ab75d87b34779dbab922bbc62
Gerrit-Change-Number: 17575
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Attila Jeges <[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: Tue, 13 Jul 2021 08:44:51 +0000
Gerrit-HasComments: Yes

Reply via email to