You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Attila Jeges (Code Review)" <ge...@cloudera.org> on 2021/07/07 14:04:52 UTC
[Impala-ASF-CR] IMPALA-10732: Use consistent DDL for specifying Iceberg partitions
Attila Jeges 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)
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
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 BUCKET and TRUNCATE transports?
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?
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 String -> TIcebergPartitionTransformType immutable map would make the code shorter and simpler.
--
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 <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 14:04:52 +0000
Gerrit-HasComments: Yes