Skip to content

fix: discard parquet bounds when row group statistics are missing - #25228

Merged
jayzhan211 merged 6 commits into
apache:mainfrom
haohuaijin:fix/parquet-missing-row-group-bounds
Sep 13, 2026
Merged

jayzhan211 merged 6 commits into
apache:mainfrom
haohuaijin:fix/parquet-missing-row-group-bounds

Conversation

@haohuaijin

@haohuaijin haohuaijin commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #25226.

Rationale for this change

MIN/MAX queries can return incorrect results when a Parquet file contains a mix of row groups with and without bounds. A missing bound is converted to a NULL in the per-row-group statistics array. Aggregating that array skips the NULL, allowing another group's endpoint to be reported as an exact bound for the entire file. The optimizer can then answer the aggregate from incomplete statistics.

For example, a file contains ["a", "b"] with statistics and two 8192-character strings starting with z without statistics. SELECT LENGTH(MIN(a)), LENGTH(MAX(a)) previously returned 1, 1; the correct result is 1, 8192.

What changes are included in this PR?

  • Invalidate a file-level bound when any row group has no converted bound and may contain non-NULL values. Return Precision::Absent so that incomplete statistics cannot establish the file's extrema.
  • Handle minimum and maximum independently, preserving a usable bound when only the other endpoint is missing.
  • Preserve bounds from other groups when the group with no bound is empty or is proven all NULL. The all-NULL check requires both the column value count and the explicitly recorded NULL count to equal the row count.
  • Express the active-accumulator precondition with expect: callers already check for Some, and the path that clears the accumulator returns immediately. This removes an unreachable fallback return.

What is the testing strategy for this PR?

test_min_max_require_complete_row_group_bounds covers absent statistics, independently missing minimum/maximum values, unknown and partial NULL counts, empty groups, all-NULL groups, and complete bounds. Every case runs in both row-group orders.

parquet_missing_bounds.slt verifies both the stored string lengths and the aggregate results. Its fixture is generated at runtime using public Rust Parquet writer APIs, enabling chunk statistics for the first row group and disabling them for the second. Disabling statistics is deliberate: parquet-rs truncates long extrema rather than omitting them. No generated Parquet file is committed and no footer is manually edited.

Local validation:

  • All 10 tests in metadata::tests::statistics_tests passed.
  • parquet_missing_bounds.slt passed with the fix. Running the same fixture and SQL test against unmodified main reproduced the incorrect 1, 1 result.
  • LLVM coverage confirmed that the removed fallback return had zero executions and that the replacement precondition assertion was exercised 36 times by the statistics tests. The updated Codecov patch percentage remains subject to the next CI run.

Are there any user-facing changes?

MIN/MAX queries return correct results for Parquet files with incomplete row-group bounds. Valid statistics remain usable for each independently complete endpoint, including when other groups are empty or all NULL. There are no public API changes.

@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) datasource Changes to the datasource crate labels Sep 12, 2026
@haohuaijin haohuaijin changed the title Fix/parquet missing row group bounds fix: discard parquet bounds when row group statistics are missing Sep 12, 2026
@codecov-commenter

codecov-commenter commented Sep 12, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.41176% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.95%. Comparing base (bb21f51) to head (74a6279).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/datasource-parquet/src/metadata.rs 99.13% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25228      +/-   ##
==========================================
+ Coverage   81.93%   81.95%   +0.01%     
==========================================
  Files        1133     1133              
  Lines      423529   423993     +464     
  Branches   423529   423993     +464     
==========================================
+ Hits       347032   347495     +463     
+ Misses      55907    55904       -3     
- Partials    20590    20594       +4     

☔ 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.

@haohuaijin
haohuaijin marked this pull request as ready for review September 12, 2026 12:48

@jayzhan211 jayzhan211 left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @haohuaijin Few non-blocking suggestions

@github-actions github-actions Bot added the auto detected api change Auto detected API change label Sep 13, 2026
@haohuaijin

Copy link
Copy Markdown
Contributor Author

Thanks @jayzhan211, I applied the suggestion.

@github-actions github-actions Bot removed the auto detected api change Auto detected API change label Sep 13, 2026
@jayzhan211
jayzhan211 added this pull request to the merge queue Sep 13, 2026
@jayzhan211

Copy link
Copy Markdown
Contributor

Thanks @haohuaijin

Merged via the queue into apache:main with commit 9082d6b Sep 13, 2026
41 checks passed
@haohuaijin
haohuaijin deleted the fix/parquet-missing-row-group-bounds branch September 13, 2026 04:07
Omega359 pushed a commit to Omega359/arrow-datafusion that referenced this pull request Oct 11, 2026
…ache#25228)

## Which issue does this PR close?

Closes apache#25226.

## Rationale for this change

MIN/MAX queries can return incorrect results when a Parquet file
contains a mix of row groups with and without bounds. A missing bound is
converted to a NULL in the per-row-group statistics array. Aggregating
that array skips the NULL, allowing another group's endpoint to be
reported as an exact bound for the entire file. The optimizer can then
answer the aggregate from incomplete statistics.

For example, a file contains `["a", "b"]` with statistics and two
8192-character strings starting with `z` without statistics. `SELECT
LENGTH(MIN(a)), LENGTH(MAX(a))` previously returned `1, 1`; the correct
result is `1, 8192`.

## What changes are included in this PR?

- Invalidate a file-level bound when any row group has no converted
bound and may contain non-NULL values. Return `Precision::Absent` so
that incomplete statistics cannot establish the file's extrema.
- Handle minimum and maximum independently, preserving a usable bound
when only the other endpoint is missing.
- Preserve bounds from other groups when the group with no bound is
empty or is proven all NULL. The all-NULL check requires both the column
value count and the explicitly recorded NULL count to equal the row
count.
- Express the active-accumulator precondition with `expect`: callers
already check for `Some`, and the path that clears the accumulator
returns immediately. This removes an unreachable fallback return.

## What is the testing strategy for this PR?

`test_min_max_require_complete_row_group_bounds` covers absent
statistics, independently missing minimum/maximum values, unknown and
partial NULL counts, empty groups, all-NULL groups, and complete bounds.
Every case runs in both row-group orders.

`parquet_missing_bounds.slt` verifies both the stored string lengths and
the aggregate results. Its fixture is generated at runtime using public
Rust Parquet writer APIs, enabling chunk statistics for the first row
group and disabling them for the second. Disabling statistics is
deliberate: parquet-rs truncates long extrema rather than omitting them.
No generated Parquet file is committed and no footer is manually edited.

Local validation:

- All 10 tests in `metadata::tests::statistics_tests` passed.
- `parquet_missing_bounds.slt` passed with the fix. Running the same
fixture and SQL test against unmodified main reproduced the incorrect
`1, 1` result.
- LLVM coverage confirmed that the removed fallback return had zero
executions and that the replacement precondition assertion was exercised
36 times by the statistics tests. The updated Codecov patch percentage
remains subject to the next CI run.

## Are there any user-facing changes?

MIN/MAX queries return correct results for Parquet files with incomplete
row-group bounds. Valid statistics remain usable for each independently
complete endpoint, including when other groups are empty or all NULL.
There are no public API changes.

---------

Co-authored-by: Jay Zhan <[email protected]>
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 sqllogictest SQL Logic Tests (.slt) v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing Parquet row-group statistics can produce incorrect file MIN/MAX

4 participants