JeonDaehong commented on code in PR #18622:
URL: https://github.com/apache/iceberg/pull/18622#discussion_r4360131045
##########
api/src/main/java/org/apache/iceberg/types/Comparators.java:
##########
@@ -46,7 +47,7 @@ private Comparators() {}
.put(Types.TimestampNanoType.withZone(), Comparator.naturalOrder())
.put(Types.TimestampNanoType.withoutZone(),
Comparator.naturalOrder())
.put(Types.StringType.get(), Comparators.charSequences())
- .put(Types.UUIDType.get(), Comparator.naturalOrder())
+ .put(Types.UUIDType.get(), Comparators.uuids())
Review Comment:
This also changes `PartitionSummary`, which builds the manifest-list
partition bounds with `Comparators.forType`. The read side compares them
through `lit.comparator()`, which is still signed for UUID.
`identity(u)` table, two files with `u = 00000000-...-0001` and `u =
c0000000-...-0000`, `newScan().filter(equal("u", x)).planFiles()`:
| filter | main | this PR |
|---|---|---|
| `u = 00...01` | 1 file | 0 files |
| `u = c0...00` | 1 file | 0 files |
The new summary is lower=`00...01`, upper=`c0...`. The signed literal
compare sees `upper < value` for both lookups and skips the manifest. On main
both sides are signed, so this works today.
##########
core/src/test/java/org/apache/iceberg/DeleteFileIndexTestBase.java:
##########
@@ -796,4 +797,87 @@ public void testInvalidDVSequenceNumber() {
.isInstanceOf(ValidationException.class)
.hasMessageContaining("must be greater than or equal to data file
sequence number");
}
+
+ @TestTemplate
+ public void testEqualityDeleteWithUuidKeyAttachesToDataFile() {
Review Comment:
Could we also have a scan-level test with an `identity(u)` partition (or a
`ManifestEvaluator` test)? This one only goes through `DeleteFileIndex`, so it
wouldn't catch the manifest pruning change above.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
