Repository navigation
fix(writer): bound a parquet file by its truncated row group statistics - #3537
Open
sankarreddy-atlan wants to merge 4 commits into
Open
sankarreddy-atlan wants to merge 4 commits into
sankarreddy-atlan wants to merge 4 commits into
Conversation
parquet-rs truncates string and binary column chunk statistics to 64 bytes and marks them inexact. MinMaxColAggregator skipped inexact statistics, so in a file with several row groups the file bound could exclude a value of the file, and bounds-based pruning dropped rows. A truncated string or binary min is a prefix and a truncated max is incremented, so both are valid bounds and are now used. A truncated value of another type, or a row group without statistics, leaves the column without a file bound on that side, as Java's ParquetMetrics does.
Author
|
Hello @kevinjqliu , Could you please review this ? |
JeonDaehong
approved these changes
Oct 28, 2026
JeonDaehong
left a comment
JeonDaehong
left a comment
Contributor
There was a problem hiding this comment.
Ran a probe on f68070f: 1500 files of random strings and binaries (1 to 4 byte UTF-8 including U+FFFF and U+10FFFF, binaries with runs of 0xFF, 1 to 4 rows per row group). With this PR every value falls inside the file's bounds. On e4f928a, 363 files got a string lower bound and 458 a string upper bound that excluded a value in the file (392 and 33 for binary). The parquet_writer tests pass too.
On #3464: agreed, a single-file position delete still gets unequal file_path bounds after truncation, so they can't tell which data file it refers to. I'll follow up on that on the position delete writer side.
…ed-stats-bounds # Conflicts: # crates/iceberg/src/writer/file_writer/parquet_writer.rs
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
What changes are included in this PR?
MinMaxColAggregatornow uses truncated string and binary statistics. A truncated min is a prefix, and parquet-rs increments a truncated max, so both are valid bounds.These cases leave the column with no file bound on that side:
fixedlonger than 64 bytes. Its length is wrong.ParquetMetrics.Long strings in single row group files now get the 64-byte truncated bounds. This does not fix the position delete
file_pathcase in #3464, because the lower and upper bounds are still not equal.Are these changes tested?
Yes.
test_truncated_statistics_bound_the_filewrites two row groups. A 100-byte string is the min of one and another is the max of the other. It fails on main with lower bound"b".fixedcolumn gets no bounds, even when a later row group has an exact max. A row group without statistics drops both bounds, even when a later row group has values.ParquetWritercannot write a file where only some row groups have statistics, so the writer loop's call todrop_boundshas no end-to-end test.AI Disclosure
This PR was prepared with AI assistance and reviewed/tested by the author.