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.
CopilotAI lite review requested due to automatic review settings August 14, 2026 15:06
@github-actionsgithub-actionsBot added core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) datasource Changes to the datasource crate labels Aug 14, 2026

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

FileDescription
datafusion/datasource-parquet/src/opener/mod.rsAdds 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.rsUpdates 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.sltAdds 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-lucaszhuqi-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
ContributorAuthor

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 linesPatch %Lines
datafusion/datasource-parquet/src/opener/mod.rs94.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-55Aug 14, 2026
35 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coreCore DataFusion cratedatasourceChanges to the datasource cratesqllogictestSQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@zhuqi-lucas@codecov-commenter@alamb