fix: preserve required ordering in limit pushdown - #24231
fix: preserve required ordering in limit pushdown#24231Himanshu-2005-code wants to merge 4 commits into
Conversation
|
Local validation completed for the change:
The implementation now preserves |
|
I’ve finished the fix and regression coverage for the order-sensitive limit issue. Summary: preserved required_ordering when limits are rebuilt during limit pushdown cargo check -p datafusion-physical-optimizer — passed |
|
@Himanshu-2005-code Is there a GitHub issue for this bug? Please follow the PR template, and include more details about the user-visible behavior that is being changed and why. Thank you! |
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #24231 +/- ##
==========================================
- Coverage 81.01% 81.01% -0.01%
==========================================
Files 1106 1106
Lines 384104 384129 +25
Branches 384104 384129 +25
==========================================
+ Hits 311194 311209 +15
- Misses 54566 54573 +7
- Partials 18344 18347 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Which issue does this PR close?
required_ordering, losing order-sensitivity for later passes #24215Rationale for this change
Queries using
ORDER BY ... LIMITcan return incorrect results after the physical plan is rewritten by limit or sort pushdown.When an order-sensitive limit is recreated during optimization, the information that the limit depends on a particular ordering can be lost. If the plan is optimized again later, the limit may then be pushed down without preserving that ordering, allowing the underlying scan to return rows in a different order.
This can cause
ORDER BY ... LIMITqueries to return different or incorrect rows.What changes are included in this PR?
required_orderingwhenGlobalLimitExecandLocalLimitExecnodes are recreated during limit pushdown.Are these changes tested?
Yes.
cargo check -p datafusion-physical-optimizercargo clippy -p datafusion-physical-optimizer --all-targets -- -D warningscargo fmt --all -- --checkcargo test -p datafusion-physical-optimizerlimit_pushdownregression testsAll local tests and checks pass.
Are there any user-facing changes?
Yes.
This fixes incorrect query results for order-sensitive
LIMIToperations after physical-plan rewrites or repeated optimization.Queries such as
ORDER BY ... LIMITwill retain the required ordering information when limits are pushed down and recreated, preventing later optimization passes from treating the limit as order-insensitive.