Skip to content

fix: preserve sorts before limits when projections hide sort keys - #26763

Merged
jayzhan211 merged 1 commit into
apache:mainfrom
kumarUjjawal:kumar/preserve-sort-before-offset
Oct 26, 2026
Merged

jayzhan211 merged 1 commit into
apache:mainfrom
kumarUjjawal:kumar/preserve-sort-before-offset

Conversation

@kumarUjjawal

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #23534.

This follows the closed, unmerged #23744.

Rationale for this change

A grouped query over ORDER BY sort_key OFFSET 1 can return sorted_first instead of physical_second. A projection hides the sort key, so the physical optimizer drops the sort even though it determines which rows the offset retains.

What changes are included in this PR?

Preserve linked sorts through operators that maintain input order, including when a projection removes their ordering metadata. Remove superseded sorts when enforcing a new ordering, and stop sort removal at explicit limits and operators with embedded fetch limits.

Add SQL and physical optimizer regressions for projections, intervening filters, local and global limits, replacement sorts, and optimizer idempotence. An execution test verifies that an embedded filter limit still selects the correct rows after one or two optimizer passes.

What is the testing strategy for this PR?

  • The ordered-offset SQL regression and embedded-fetch execution regression fail without their respective fixes.
  • The full extended suite passes: 12,987 Rust tests and 542 SQL files.
  • cargo fmt --all, cargo clippy --all-targets --all-features -- -D warnings, and the complete ./dev/rust_lint.sh suite pass.
  • Compared physical_plan_struct_join_agg_sort and physical_sorted_union_order_by_10_int64 against main at 350ca76505c9513d16e7dd9910bcd9bde2c911ed, using 30 samples each. Neither benchmark showed a statistically significant performance change.

Are there any user-facing changes?

Queries retain the rows selected by ordered LIMIT/OFFSET when a projection hides the sort key. No public API changes.

@github-actions github-actions Bot added optimizer Optimizer rules core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels Oct 26, 2026

@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 @kumarUjjawal 👍🏻

@jayzhan211
jayzhan211 added this pull request to the merge queue Oct 26, 2026
Merged via the queue into apache:main with commit 16c1f26 Oct 26, 2026
45 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 optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Returns the wrong grouped row when an outer GROUP BY consumes a subquery containing ORDER BY ... OFFSET.

2 participants