Skip to content

[branch-55]: don't runtime-prune row groups while a page-index RowSelection is live (#24355) - #24374

Merged
zhuqi-lucas merged 1 commit into
apache:branch-55from
zhuqi-lucas:backport-24359-branch-55
Aug 14, 2026
Merged

[branch-55]: don't runtime-prune row groups while a page-index RowSelection is live (#24355)#24374
zhuqi-lucas merged 1 commit into
apache:branch-55from
zhuqi-lucas:backport-24359-branch-55

Conversation

@zhuqi-lucas

Copy link
Copy Markdown
Contributor

Backport of #24359 to branch-55 for the 55.0.0 release, per @timsaucer's request in #22393. Stacks cleanly on the already-merged #24368 (#24354 backport).

Which issue does this PR close?

Rationale

With pushdown_filters=true + a TopK dynamic filter, the runtime row-group pruner rebuilds the push decoder via into_builder().with_row_groups(...), which drops row groups without slicing the carried flat page-index RowSelection to match — a dropped RG's selectors are then applied to the next surviving RG, silently returning wrong rows (no error). The fix declines to build the runtime RowGroupPruner when a row selection is present (correctness over the pruning optimization); the proper fix that keeps both is tracked upstream in apache/arrow-rs#10624 / #24358.

Notes

cc @timsaucer @alamb @adriangb

…lection is live (apache#24355)

Backport of apache#24359 to branch-55.

The runtime row-group pruner rebuilds the push decoder via
into_builder().with_row_groups(...), which drops row groups without slicing
the carried flat page-index RowSelection to match — a dropped RG's selectors
are then applied to the next surviving RG, silently returning wrong results.
Decline to build the runtime RowGroupPruner when a row selection is present
(mirroring reorder_by_statistics). Proper fix tracked in arrow-rs apache#10624 / apache#24358.
Copilot AI lite review requested due to automatic review settings August 14, 2026 15:06
@github-actions github-actions Bot added core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) datasource Changes to the datasource crate labels Aug 14, 2026
@zhuqi-lucas
zhuqi-lucas requested review from alamb and timsaucer August 14, 2026 15:06

Copilot AI 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.

Pull request overview

Backports the fix for DataFusion issue #24355 to branch-55 by preventing parquet runtime row-group pruning when a page-index (or externally supplied) RowSelection is present, avoiding silent wrong-results caused by misaligned selections after decoder rebuilds.

Changes:

  • Disable construction of the runtime RowGroupPruner when PreparedAccessPlan.row_selection.is_some() to prevent into_builder().with_row_groups(...) from dropping row groups without remapping the carried selection.
  • Add an SLT regression test reproducing the wrong-results scenario (a >= 50 with TopK + page-index pruning) and asserting correct output.
  • Update/rename the Rust integration test to assert the new intended behavior (page-index pruning still runs, runtime dynamic RG pruning is skipped), and adjust the #24352 regression test to disable page-index reading so it still exercises runtime pruning.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
datafusion/datasource-parquet/src/opener/mod.rs Adds a guard to skip runtime dynamic row-group pruning while a RowSelection is live, preventing silent misalignment bugs.
datafusion/core/tests/parquet/dynamic_row_group_pruning.rs Updates regression coverage to reflect the new behavior (pruner disabled under row selection) and keeps #24352 test exercising runtime pruning.
datafusion/sqllogictest/test_files/dynamic_row_group_pruning.slt Adds an end-to-end SLT regression reproducer for the page-index RowSelection + TopK dynamic pruning wrong-results case.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@zhuqi-lucas zhuqi-lucas changed the title Backport #24359 to branch-55: don't runtime-prune row groups while a page-index RowSelection is live (#24355) [branch-55]: don't runtime-prune row groups while a page-index RowSelection is live (#24355) Aug 14, 2026
@zhuqi-lucas

Copy link
Copy Markdown
Contributor Author

Thanks @alamb for review, merged now.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.14%. Comparing base (26c0204) to head (f0d664d).

Files with missing lines Patch % Lines
datafusion/datasource-parquet/src/opener/mod.rs 94.11% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           branch-55   #24374      +/-   ##
=============================================
- Coverage      81.14%   81.14%   -0.01%     
=============================================
  Files           1110     1110              
  Lines         386182   386184       +2     
  Branches      386182   386184       +2     
=============================================
- Hits          313370   313363       -7     
- Misses         54344    54352       +8     
- Partials       18468    18469       +1     

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

@zhuqi-lucas
zhuqi-lucas merged commit f51b9ea into apache:branch-55 Aug 14, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate datasource Changes to the datasource crate sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants