Commit cd7d208
Restore Sort unparser guard for correct ORDER BY placement (apache#20658)
## Which issue does this PR close?
- closes apache#20905
- closes apache#20923
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:
```rust
if select.already_projected() {
return self.derive_with_dialect_alias("derived_sort", plan, relation, false, vec![]);
}
```1 parent 1563ef8 commit cd7d208
3 files changed
Lines changed: 56 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
499 | 499 | | |
500 | 500 | | |
501 | 501 | | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
502 | 513 | | |
503 | 514 | | |
504 | 515 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
223 | 223 | | |
224 | 224 | | |
225 | 225 | | |
226 | | - | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
227 | 235 | | |
228 | 236 | | |
229 | 237 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1740 | 1740 | | |
1741 | 1741 | | |
1742 | 1742 | | |
| 1743 | + | |
| 1744 | + | |
| 1745 | + | |
| 1746 | + | |
| 1747 | + | |
| 1748 | + | |
| 1749 | + | |
| 1750 | + | |
| 1751 | + | |
| 1752 | + | |
| 1753 | + | |
| 1754 | + | |
| 1755 | + | |
| 1756 | + | |
| 1757 | + | |
| 1758 | + | |
| 1759 | + | |
| 1760 | + | |
| 1761 | + | |
| 1762 | + | |
| 1763 | + | |
| 1764 | + | |
| 1765 | + | |
| 1766 | + | |
| 1767 | + | |
| 1768 | + | |
| 1769 | + | |
| 1770 | + | |
| 1771 | + | |
| 1772 | + | |
| 1773 | + | |
| 1774 | + | |
| 1775 | + | |
| 1776 | + | |
| 1777 | + | |
| 1778 | + | |
1743 | 1779 | | |
1744 | 1780 | | |
1745 | 1781 | | |
| |||
0 commit comments