Skip to content

fix: preserve local TopK limits during exact sort pushdown - #26736

Draft
geoffreyclaude wants to merge 1 commit into
apache:mainfrom
geoffreyclaude:fix/exact-sort-pushdown-local-limit
Draft

geoffreyclaude wants to merge 1 commit into
apache:mainfrom
geoffreyclaude:fix/exact-sort-pushdown-local-limit

Conversation

@geoffreyclaude

@geoffreyclaude geoffreyclaude commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #26739.

Rationale for this change

Sort pushdown can make an ORDER BY ... LIMIT query fail plan validation when a source guarantees the requested ordering but cannot apply the row limit itself.

A partition-preserving TopK keeps up to k rows per partition. Removing its sort must retain that local limit. The standalone exact-pushdown path always inserted GlobalLimitExec instead, which requires a single input partition. With a four-partition source, the resulting plan fails with a SinglePartition distribution error.

The direct merge/sort path already uses a local fallback. The standalone path needs to preserve the same limit scope.

What changes are included in this PR?

When the source cannot absorb the fetch, choose LocalLimitExec if the removed sort preserves partitions, and GlobalLimitExec otherwise.

What is the testing strategy for this PR?

The SQL regression uses a native running-sum window over a four-partition source, with an explicit ORDER BY group_id, value LIMIT 3. The custom source only supplies data and accepts exact sort pushdown; it cannot apply a limit.

The window is needed to reach the affected optimization: PARTITION BY requires redistributing this source's rows by group, and the outer LIMIT lets the optimizer add a local TopK before that redistribution. Without the window, this query takes the already-correct merge/sort path.

The test checks the three expected rows and uses EXPLAIN to verify that LocalLimitExec remains below the repartition. Without the fix, the query fails plan validation. Removing the limit entirely would fail the plan assertion.

Validation: the focused SQL file, existing PushdownSort integration tests, formatting, and Clippy with the standard CI feature set (avro,integration-tests,extended_tests).

Are there any user-facing changes?

Affected queries execute successfully while preserving the per-partition and overall row limits. No public API changes.

Developed with AI assistance.

@github-actions github-actions Bot added optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) labels Oct 25, 2026
@geoffreyclaude
geoffreyclaude force-pushed the fix/exact-sort-pushdown-local-limit branch from 6ce49ef to 5c86f33 Compare October 25, 2026 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TopK queries can fail plan validation after exact sort pushdown

1 participant