Arnab Karmakar has posted comments on this change. ( 
https://gerrit.cloudera.org/24973 )

Change subject: IMPALA-15365: Iceberg UUID partition transform support
......................................................................


Patch Set 2:

(9 comments)

Thanks for the comments!

https://gerrit.cloudera.org/#/c/24973/1//COMMIT_MSG
Commit Message:

https://gerrit.cloudera.org/#/c/24973/1//COMMIT_MSG@9
PS1, Line 9: Adds partition transform support for the Iceberg UUID type. The 
Icebe
> Please mention it is not an Impala limitation, but the spec only allows the
Done


https://gerrit.cloudera.org/#/c/24973/1//COMMIT_MSG@13
PS1, Line 13: and ALTER TABLE ... SET PARTITION SPEC
> UUIDs use unsigned byte-wise comparison: https://github.com/apache/iceberg/
Done


https://gerrit.cloudera.org/#/c/24973/1//COMMIT_MSG@14
PS1, Line 14:
            :
> Please mention that this is a bug in the Iceberg Java library: https://gith
Done


https://gerrit.cloudera.org/#/c/24973/1/fe/src/main/java/org/apache/impala/common/IcebergPartitionPredicateConverter.java
File 
fe/src/main/java/org/apache/impala/common/IcebergPartitionPredicateConverter.java:

https://gerrit.cloudera.org/#/c/24973/1/fe/src/main/java/org/apache/impala/common/IcebergPartitionPredicateConverter.java@71
PS1, Line 71: partition. != and NOT IN only prune manifests whose summary holds 
a single v
> Holds for writers that use the Java library (Impala, Trino, Spark). A write
Done


https://gerrit.cloudera.org/#/c/24973/1/fe/src/main/java/org/apache/impala/common/IcebergPartitionPredicateConverter.java@73
PS1, Line 73:  with the Iceberg Java library's signed c
> != and NOT_IN could be allowed as well.
Done


https://gerrit.cloudera.org/#/c/24973/1/fe/src/main/java/org/apache/impala/common/IcebergPartitionPredicateConverter.java@74
PS1, Line 74:  use the library (Impa
> Instead of using canUsePartitionKeyScan(), consider extracting logic to a n
Done


https://gerrit.cloudera.org/#/c/24973/1/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java
File fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java:

https://gerrit.cloudera.org/#/c/24973/1/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@475
PS1, Line 475: The Iceberg Java library compares UUID
> Same as in commit message: it is a bug in the Iceberg Java Library, not in
Done


https://gerrit.cloudera.org/#/c/24973/1/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@476
PS1, Line 476:     // use unsigned byte order, so Iceberg could skip data files 
that contain matching
> When UUID is part of identity-partitioning, we could still push down = and
Thanks for pointing it out! They can be pushed down with the narrowed down 
forms. The identity partition would be simpler but since bucket transform might 
need some changes in planning and its own set of tests, it'd be cleaner to do 
it in a follow-up patch. Filed IMPALA-15476.


https://gerrit.cloudera.org/#/c/24973/1/testdata/data/README
File testdata/data/README:

https://gerrit.cloudera.org/#/c/24973/1/testdata/data/README@1035
PS1, Line 1035: iceberg_uuid_test_part
> Each of the 4 appends writes a single-file manifest, so the partition summa
Done



--
To view, visit https://gerrit.cloudera.org/24973
To unsubscribe, visit https://gerrit.cloudera.org/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dc3af4418285d6ea1d489912419be470e668f29
Gerrit-Change-Number: 24973
Gerrit-PatchSet: 2
Gerrit-Owner: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Sat, 03 Oct 2026 17:47:03 +0000
Gerrit-HasComments: Yes

Reply via email to