Skip to content

Restore Sort unparser guard for correct ORDER BY placement#20658

Open
krinart wants to merge 3 commits intoapache:mainfrom
spiceai:viktor/fix-unparser-origin
Open

Restore Sort unparser guard for correct ORDER BY placement#20658
krinart wants to merge 3 commits intoapache:mainfrom
spiceai:viktor/fix-unparser-origin

Conversation

@krinart
Copy link
Contributor

@krinart krinart commented Mar 3, 2026

Which issue does this PR close?

Restore the already_projected() guard in the Sort case of select_to_sql_recursively. Without it, queries with ORDER BY expressions generate invalid SQL when the Sort node follows an outer Projection.

Problem

Two regressions in the DuckDB federation unparser after upgrading to DataFusion 52:

  1. clickbench q25: Queries like SELECT "SearchPhrase" ... ORDER BY to_timestamp("EventTime") LIMIT 10 produce ORDER BY outside the subquery, referencing table names ("hits") that are out of scope.
  2. tpcds q36: ORDER BY with GROUPING() expressions loses the derived_sort subquery alias, placing sort references outside their valid scope.

Root Cause

DF52's optimizer merges Limit into Sort as a fetch parameter, changing the plan shape from Projection → Limit → Sort → ... to Projection → Sort(fetch) → .... The Sort case previously had a guard that wrapped the sort into a derived_sort subquery when already_projected() was true. This guard was removed in DF52, causing ORDER BY and LIMIT to be placed on the outer query where inner table references are invalid.

Fix

Re-add the guard at the top of the Sort match arm in select_to_sql_recursively:

 if select.already_projected() {
     return self.derive_with_dialect_alias("derived_sort", plan, relation, false, vec![]);
 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant