perf(table): prune equality deletes by data-file metrics - #1960
Conversation
Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
0b5b5aa to
e674a01
Compare
zeroshade
left a comment
There was a problem hiding this comment.
This adds conservative metric-based equality-delete pruning without altering delete applicability. Approving — and recording the invariants below explicitly, because they're what makes this safe and whoever touches this code next needs them.
An over-aggressive prune here silently resurrects deleted rows, so I went through the uncertainty paths rather than the happy path.
The core invariant: metric absent means cannot prune, never safe to prune
table/equality_delete_index.go:541-617 only drops a candidate after both typed bounds decode and prove the ranges disjoint. Candidates are retained for missing, partial, inverted, and malformed bounds, and for unknown, dropped, and nested equality fields. Every uncertainty resolves toward reading the delete file.
Type promotion — the subtle one
Older promoted-width bounds (4-byte where the current type is wider) fail to decode and fall through to retain. That's the correct outcome: the alternative — comparing a misinterpreted value — would prune on garbage. This is the difference between safe and silently wrong across schema evolution, so it's worth keeping that fall-through intact if this code is refactored. Same-scale decimals compare correctly.
NaN
Float and double range pruning at :683 requires NaN counts to be present and zero on both the data and delete side before any range comparison. NaN breaking ordering comparisons is the classic way this optimisation goes wrong, and gating on explicit zero counts on both sides is the right bar.
Nulls
The optional-field checks at :619-681 preserve the candidate when null or value counts are missing or negative, and use null-only proofs solely where the counts actually establish presence or absence. Equality deletes on NULL can't be decided from min/max, and they aren't.
Multi-column keys
A single proven-disjoint conjunct is sufficient, which is correct rather than over-aggressive: an equality-delete row must match a data row on every equality column, so if one column's ranges are disjoint, no data row can match any delete row in that file. I checked this against the spec's equality-delete semantics and the Java DeleteFileIndex equivalent.
Existing gating preserved
Strict sequence-number ordering (delete sequence > data sequence) and partition/global routing are unchanged, so this only removes candidates that provably cannot match — it doesn't alter which delete files apply to which data files.
Evidence: BenchmarkEqualityDeleteIndexMetrics is committed, with a 10,000-data-file / 10,000-delete-file workload going from ~1.4–1.7 s and 6.66 GB to ~0.93–1.11 s and 52.5 MB. That allocation reduction is the headline.
No findings. CI green (15/15), all 4 commits signed off.
This review was drafted by an AI-assisted tool and confirmed by an Apache Iceberg Go maintainer, who has read the findings and signed off. If something feels off, please reply on the PR and a maintainer will follow up.
More on how to contribute to Apache Iceberg Go: CONTRIBUTING.md
What
DeleteFileIndex.Java reference: https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/DeleteFileIndex.java
Why
With many equality deletes in one partition, most delete files can be ruled out from a data file by its metrics. This reduces the delete files attached to each
FileScanTask, which also reduces downstream delete-file reads.Benchmark
Workload: 10,000 data files, 10,000 equality deletes, one partition, 100 disjoint key-range groups. Each data file overlaps 100 delete files (1%).
The benchmark was run with
go test ./table -run ^$ -bench ^BenchmarkEqualityDeleteIndexMetrics$ -benchtime=1x -count=5on an Apple M1 Pro.Checks
go test ./...go test -race ./tablego vet ./...✅