Skip to content

fix: honor Parquet byte-array statistics ordering - #24525

Open
sunchao wants to merge 1 commit into
apache:mainfrom
sunchao:dev/chao/codex/parquet-statistics-order
Open

fix: honor Parquet byte-array statistics ordering#24525
sunchao wants to merge 1 commit into
apache:mainfrom
sunchao:dev/chao/codex/parquet-statistics-order

Conversation

@sunchao

@sunchao sunchao commented Aug 20, 2026

Copy link
Copy Markdown
Member

Why are the changes needed?

Which issue does this PR close?

Part of #10586. This fixes the unsigned string/binary byte-array case; it does not claim to resolve every Parquet ordering issue. It is also the correctness prerequisite for #24526.

Rationale for this change

A Parquet scan can skip a file, row group, or page when its statistics prove that no row can satisfy the filter. If those statistics use a different comparison order from the query, that proof is invalid: DataFusion can silently discard a row that should be returned.

For example, suppose a Parquet row group contains 'aé', 'az', and 'b':

SELECT s FROM t WHERE s = 'az';
-- Expected: one row containing 'az'

Parquet's deprecated byte-array min/max fields use signed comparison, whereas Arrow compares strings using unsigned UTF-8 bytes. The same values therefore have two different orders:

Comparison Values in ascending order Minimum / maximum
Legacy signed-byte order aé < az < b / b
Arrow's unsigned-byte order az < aé < b az / b

The first UTF-8 byte of é is 0xC3: it sorts before z's 0x7A as a signed byte, but after it as an unsigned byte. If DataFusion interprets the legacy interval ['aé', 'b'] using Arrow's ordering, it sees 'az' < 'aé' and can incorrectly skip the row group. Merely checking that min <= max does not help: the two reported endpoints are still in ascending order.

The newer min_value/max_value fields and page-index bounds also need an ordering declaration that the reader understands. A missing or unknown column_orders entry is not enough to justify assuming Arrow's ordering. The Parquet statistics definition and logical-type ordering rules describe these distinctions.

This bug is independently observable with an ordinary equality filter; it does not require a large IN list.

What changes were proposed in this PR?

What changes are included in this PR?

The change makes a recognized comparison order a prerequisite for using unsigned byte-array bounds. At the point where Parquet metadata becomes Arrow statistics, DataFusion checks whether the column's physical/logical type and footer establish the expected unsigned order. It also rejects row-group bounds taken from the deprecated signed-order fields. When that evidence is missing, min/max is reported as unknown, so pruning keeps potentially matching data instead of guessing.

The rule is applied at the granularity where the statistics are used. An unsafe row group prevents DataFusion from claiming a trustworthy bound for the whole file, but it does not make other row groups' valid bounds unusable. Page-index bounds are checked against the footer independently. The same safeguards cover static and runtime row-group pruning, as well as the inverse predicates used to decide whether every row already satisfies a filter. Null counts and unrelated columns' statistics remain available.

The row-group pruning API gains a metadata-aware entry point so callers can supply the footer needed for this decision. The existing entry point remains source-compatible and behaves conservatively when that information is unavailable. Signed logical types such as decimals retain their existing behavior.

Are there any user-facing changes?

Queries no longer discard matching data because of these untrustworthy byte-array bounds. Older files, or files with an unrecognized ordering, may require more scanning. Modern files with trustworthy bounds retain min/max pruning. There is no file-format change or breaking public API change.

How was this PR tested?

Are these changes tested?

The regression uses actual serialized Parquet files containing the example above, with modern, deprecated, missing-order, and unknown-order metadata. It checks that the matching az row survives file, row-group, runtime, and page pruning, while valid statistics can still eliminate unrelated data. On unchanged Apache f1f0449a, the adapted equality regression fails because the deprecated-order case loses az; the modern-statistics control passes.

The dedicated Parquet-crate run passed 231 unit tests and four doctests. Its seven focused ordering tests also cover mixed safe/unsafe row groups, null counts, fixed-length binary and UUID, signed decimal, and logical types with undefined ordering. Formatting, all-targets/all-features Clippy with warnings denied, and ./dev/rust_lint.sh passed. The extended workspace run passed 10,666 Rust tests, with eight ignored, and all 503 SQL-logic files.

The existing metadata benchmark was run on Apache f1f0449a and this patch using the same valid modern-footer fixture. Across nine full-statistics cases there was no material regression; the largest case, with 256 columns and 128 row groups, measured 1.330 ms before and 1.332 ms after. Both runs used Rust 1.97.0, release-nonlto, 20 samples, and separate build directories on an Apple M5 Max.

Validation commands
cargo test --locked --profile ci -p datafusion-datasource-parquet

cargo bench --locked --profile release-nonlto -p datafusion-datasource-parquet \
  --bench parquet_metadata_statistics -- \
  metadata_full --sample-size 20 --warm-up-time 0.5 --measurement-time 1 --noplot

RUST_BACKTRACE=1 cargo test --locked --profile ci \
  --exclude datafusion-examples --exclude datafusion-benchmarks --exclude datafusion-cli \
  --workspace --lib --tests --bins \
  --features avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption

@github-actions github-actions Bot added the datasource Changes to the datasource crate label Aug 20, 2026
@sunchao
sunchao marked this pull request as ready for review August 20, 2026 16:25
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.98658% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.31%. Comparing base (f1f0449) to head (10aafae).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
...afusion/datasource-parquet/src/row_group_filter.rs 96.10% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24525      +/-   ##
==========================================
+ Coverage   81.27%   81.31%   +0.04%     
==========================================
  Files        1116     1117       +1     
  Lines      395017   396039    +1022     
  Branches   395017   396039    +1022     
==========================================
+ Hits       321055   322056    +1001     
+ Misses      55166    55159       -7     
- Partials    18796    18824      +28     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sunchao

sunchao commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

cc @alamb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

datasource Changes to the datasource crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants