sunchao opened a new pull request, #24525:
URL: https://github.com/apache/datafusion/pull/24525

   ## Why are the changes needed?
   
   ### Which issue does this PR close?
   
   Part of #10586. This addresses unsigned string/binary byte-array bounds, not 
every possible Parquet ordering issue.
   
   ### Rationale for this change
   
   Parquet's [legacy byte-array 
statistics](https://github.com/apache/parquet-format/blob/master/src/main/thrift/parquet.thrift)
 use a different ordering from Arrow's string and binary comparisons. For 
example, signed-byte bounds for `['aé', 'az', 'b']` can be `['aé', 'b']`. Those 
endpoints look valid under unsigned comparison but incorrectly exclude the 
matching value `az`. Modern-looking bounds also cannot be interpreted safely 
when the footer omits `column_orders` or contains an unknown order.
   
   As a result, an ordinary equality filter can discard matching data during 
file, row-group, or page pruning.
   
   ## What changes were proposed in this PR?
   
   ### What changes are included in this PR?
   
   - Validate the physical/logical byte-array ordering and the file footer 
before using min/max bounds.
   - Keep file-level bounds absent when an untrusted row group prevents a valid 
whole-file bound, while masking individual unsafe row-group bounds so 
trustworthy groups remain prunable.
   - Apply the same rules to static, runtime, and inverted row-group pruning 
and page-index pruning. Preserve null counts and unrelated columns' statistics.
   - Add a metadata-aware row-group pruning API; retain the existing footerless 
API conservatively. Signed logical types such as decimals retain their existing 
behavior.
   - Add real-file regressions with modern, legacy, missing-order, and 
unknown-order footers, plus type-specific tests.
   - Give the existing metadata benchmark a valid modern column-order footer so 
it continues to measure usable bounds.
   
   ### Are there any user-facing changes?
   
   Queries no longer use untrustworthy byte-array bounds to discard data. 
Readers may perform less min/max pruning on older or unknown-order files. The 
metadata-aware API is additive; existing callers remain source-compatible.
   
   ## How was this PR tested?
   
   ### Are these changes tested?
   
   The dedicated Parquet-crate run passed 231 unit tests and four doctests. 
Seven focused tests cover actual serialized legacy, missing-order, and 
unknown-order files; file/static/runtime/page pruning; null counts; safe modern 
row groups; fixed-length binary and UUID; signed decimal; and undefined logical 
types.
   
   The real-file equality regression was also run against unchanged Apache 
`f1f0449a`, with only the test adapted to the old API. It fails there because 
the deprecated-order case loses the matching `az` row at file and row-group 
pruning. The modern-statistics control passes.
   
   The existing `parquet_metadata_statistics` benchmark was run on Apache 
`f1f0449a` and this patch, with the same modern-footer fixture in each. Across 
the nine full-statistics cases there was no material regression; the largest 
case (256 columns, 128 row groups) measured 1.330 ms on main and 1.332 ms with 
this change. Both runs used Rust 1.97.0, `release-nonlto`, 20 samples, and 
separate build directories on an Apple M5 Max.
   
   ```sh
   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
   ```
   
   Formatting, all-targets/all-features Clippy with warnings denied, and 
`./dev/rust_lint.sh` passed. The required extended workspace run also passed: 
10,666 Rust tests, eight ignored, and all 503 SQL-logic files.
   
   ```sh
   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
   ```
   


-- 
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]

Reply via email to