Neuw84 opened a new pull request, #18622: URL: https://github.com/apache/iceberg/pull/18622
Closes #18619. ## Problem An equality delete keyed on a `uuid` column (most visibly a composite key like `(uuid, int)`) is never attached to any `FileScanTask.deletes()`, so none of its rows are deleted. The loaded delete set is correct; the delete file simply never reaches the reader because scan planning decides it cannot apply. Root cause is the UUID comparator. `Comparators` maps `UUIDType` to `Comparator.naturalOrder()`, i.e. `java.util.UUID.compareTo`, which compares the two halves as **signed** longs: ```java .put(Types.UUIDType.get(), Comparator.naturalOrder()) ``` But UUIDs are serialized and bounded as **16-byte big-endian** values (spec Appendix B/D), and Parquet computes a data file's min/max bounds over those bytes in **unsigned** order. So a data file's stored `[lower, upper]` uuid bounds form an unsigned interval. When that interval straddles the signed boundary — the most significant long of either bound is negative as a signed `long`, i.e. the leading byte is `>= 0x80`, which is roughly half of all random UUIDs — the signed comparator sees `lower > upper` and treats the interval as inverted. In `DeleteFileIndex.canContainEqDeletesForFile` → `rangesOverlap`, the range check then concludes the delete cannot overlap the data file and the delete is dropped. With a composite `(uuid, int)` key the false per-field result ANDs into the whole check, so the delete is excluded reliably. Single-column `(int)` / `(string)` controls work because their comparators match their byte serialization. ## Fix Order `UUIDType` with an unsigned 128-bit comparator — `Long.compareUnsigned` on the most significant bits, then the least — which matches the big-endian unsigned order the bounds are computed and serialized in. This fixes the mismatch for **every** uuid bounds comparison (delete planning, manifest pruning, `ParquetMetricsRowGroupFilter`, residual evaluation), not just this one call site. It is a change to UUID sort order, but toward the spec-mandated big-endian unsigned order; the only behavior that relied on the previous signed order was relying on the bug. ## Tests - `TestComparators.testUuid`: adds pairs that cross the signed/unsigned boundary (e.g. `7fff.. < 8000..`, `00.. < ff..`, and an MSB tie that falls through to the least significant bits). These **fail** under the old signed comparator and pass with the fix. The pre-existing pair is kept and still holds (both of its UUIDs sort the same way signed or unsigned, which is why the bug went unnoticed). - `DeleteFileIndexTestBase.testEqualityDeleteWithUuidKeyAttachesToDataFile`: an end-to-end scan-planning test. A data file whose uuid bounds `[00..01, ff..fe]` straddle the boundary, and a `(uuid, int)` equality delete whose uuid key `c0..` lies inside the range only under unsigned ordering, must be attached (`forDataFile(...).hasSize(1)`). Fails under the old comparator (the delete is dropped), passes with the fix. Runs under every parameterized format version in the base, so it covers the local and REST scan-planning subclasses. ## Build / test note I could not run the Gradle suite locally — this environment only has JDK 25 and the Iceberg build requires JDK 17 or 21 (`This build must be run with JDK 17 or 21`). The change is small and both new tests pin the exact ordering contract; CI will exercise them under a supported JDK. -- 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]
