Support ORDER BY ALL for projected expressions - #25243
Open
osipovartem wants to merge 2 commits into
Open
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25243 +/- ##
==========================================
- Coverage 81.95% 81.94% -0.01%
==========================================
Files 1133 1133
Lines 423799 423793 -6
Branches 423799 423793 -6
==========================================
- Hits 347307 347294 -13
- Misses 55899 55907 +8
+ Partials 20593 20592 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jayzhan211
approved these changes
Sep 13, 2026
jayzhan211
left a comment
Contributor
There was a problem hiding this comment.
Thanks @osipovartem , LGTM!
jayzhan211
approved these changes
Sep 13, 2026
| # specific language governing permissions and limitations | ||
| # under the License. | ||
|
|
||
| # ORDER BY ALL expands computed expressions and aliases in select-list order. |
Contributor
There was a problem hiding this comment.
It would be nice to add
SELECT b AS a, a AS b FROM t ORDER BY ALL, plus optionally a self-join with duplicate output names.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes a planner gap where
ORDER BY ALLonly accepted raw column projections and was silently ignored for set-operation outputs.Rationale for this change
ORDER BY ALLmeans sorting by every output column in select-list order. Expanding it to the equivalent 1-based ordinal keys supports computed expressions, aliases, aggregate outputs, wildcard-expanded projections, and set-operation outputs through one planner path.The ordinal expansion also sorts already-projected columns rather than evaluating computed expressions again. Physical execution remains on DataFusion's existing vectorized
SortExec; this adds no row-wise conversion or custom physical operator.What changes are included?
OrderByKind::Allto ordinalOrderByExprs from the output widthSELECTset expressions instead of droppingORDER BY ALLAre these changes tested?
cargo +1.95.0 test -p datafusion-sqllogictest --test sqllogictests -- order_by_all.slt --test-threads 1cargo +1.95.0 test -p datafusion-sql --test sql_integration -- --test-threads 8(591 passed)cargo +1.95.0 clippy -p datafusion-sql --all-targets -- -D warningscargo +1.95.0 fmt --all -- --checkSnowflake dialect parsing support is proposed independently in apache/datafusion-sqlparser-rs#2502; this planner change is generic to every dialect that already emits
OrderByKind::All.