Skip to content

fix(unparser): inline computed projection aliases in ORDER BY expressions - #177

Merged
Jeadie merged 2 commits into
spiceai-54from
jeadie/fix-unproject-sort-expr-54
Jul 1, 2026
Merged

fix(unparser): inline computed projection aliases in ORDER BY expressions#177
Jeadie merged 2 commits into
spiceai-54from
jeadie/fix-unproject-sort-expr-54

Conversation

@Jeadie

@Jeadie Jeadie commented Jul 1, 2026

Copy link
Copy Markdown

Problem

During Datafusion unparsing, only ScalarFunction projections are reinlined for ORDER BY expression (prior work by @phillipleblanc; upstreamed in apache#16127) . Any other computed expression — e.g. a BinaryExpr such as grouping(a) + grouping(b) — fell through, leaving a bare SELECT-list alias inside the ORDER BY.

A bare alias is a valid top-level sort key, but when it appears inside a larger expression (e.g. CASE WHEN), standard-conforming engines resolve identifiers against the FROM relations only and reject it:

  • PostgreSQL: ERROR: column "computed" does not exist (SQLSTATE 42703)
  • DuckDB: Binder Error: Referenced column "computed" not found
  • MySQL: ERROR 1054 (42S22): Unknown column 'computed' in 'order clause'

Before (broken)

SELECT (t1.id + t1.age) AS computed, t1.id
FROM t1
ORDER BY CASE WHEN (computed = 0) THEN t1.id END   -- alias leaks into expression

After (valid)

SELECT (t1.id + t1.age) AS computed, t1.id
FROM t1
ORDER BY CASE WHEN ((t1.id + t1.age) = 0) THEN t1.id END   -- inlined

Fix

Generalize the re-inlining in unproject_sort_expr to any non-trivial projection expression (excluding plain Column renames, AggregateFunction, and WindowFunction).

Why this surfaced in the DataFusion 54 upgrade

Changes to OptimizeProjections in DF54 changed the produced LogicalPlan and caused this issue.

DF53: Projection → Sort → Projection → WindowAggr
DF54: Sort → Projection → WindowAggr

If a projection is already open above the Sort (already_projected() == true), the unparser wraps the whole thing in a derived_sort subquery: SELECT ... FROM (SELECT ... AS computed ... ORDER BY ...) AS derived_sort. Within this CTE, computed is a real derived-table column.

Tests

  • New regression test test_order_by_with_computed_alias_inside_expr. Verified to fail without the fix.

Jeadie added 2 commits July 1, 2026 12:27
…ions

unproject_sort_expr only re-inlined ScalarFunction projection expressions
when unprojecting sort keys. Any other computed expression (e.g. a
BinaryExpr like grouping(a) + grouping(b)) fell through, leaving a bare
SELECT-list alias inside the ORDER BY. When that alias appears inside a
larger expression (e.g. CASE WHEN), standard-conforming engines resolve
identifiers against the FROM relations only and reject it (PostgreSQL:
column "..." does not exist, SQLSTATE 42703).

Generalize the re-inlining to any non-trivial projection expression
(excluding plain Column renames, AggregateFunction and WindowFunction),
with best-effort aggregate-column resolution for the inlined expression.

Adds a regression test covering a computed alias referenced inside an
ORDER BY CASE WHEN.
@Jeadie
Jeadie marked this pull request as ready for review July 1, 2026 02:35
Copilot AI review requested due to automatic review settings July 1, 2026 02:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes SQL unparsing for ORDER BY expressions that reference SELECT-list aliases inside larger expressions (e.g. CASE WHEN ...), by re-inlining the underlying projection expression instead of emitting a bare alias identifier that standard-conforming SQL engines resolve only against FROM.

Changes:

  • Generalize unproject_sort_expr to re-inline non-trivial projection expressions (not just ScalarFunction) when referenced via Expr::Column in ORDER BY.
  • Add a regression test covering a computed (BinaryExpr) projection alias used inside an ORDER BY CASE WHEN expression.
  • Minor comment whitespace cleanup in unproject_unnest_expr.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
datafusion/sql/src/unparser/utils.rs Generalizes ORDER BY alias re-inlining to handle any non-trivial projection expression (with best-effort aggregate expression resolution).
datafusion/sql/tests/cases/plan_to_sql.rs Adds a regression test ensuring computed projection aliases are inlined when referenced inside ORDER BY expressions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Jeadie
Jeadie merged commit c94202b into spiceai-54 Jul 1, 2026
1 check passed
@Jeadie
Jeadie deleted the jeadie/fix-unproject-sort-expr-54 branch July 1, 2026 02:51
pull Bot pushed a commit to TheRakeshPurohit/spiceai that referenced this pull request Jul 1, 2026
Jeadie added a commit to spiceai/spiceai that referenced this pull request Jul 1, 2026
github-actions Bot pushed a commit to spiceai/spiceai that referenced this pull request Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants