Skip to content

Commit b3d2cb6

Browse files
adriangbclaude
andauthored
Fix ORDER BY positional reference regression with aliased aggregates (#19412)
## Which issue does this PR close? Closes #19410 ## Rationale for this change This PR fixes a regression introduced in #18831 where queries using GROUP BY with ORDER BY positional reference to an aliased aggregate fail with: ``` Error during planning: Column in ORDER BY must be in GROUP BY or an aggregate function ``` **Failing query (now fixed):** ```sql with t as (select 'foo' as x) select x, count(*) as "Count" from t group by x order by 2 desc; ``` ## What changes are included in this PR? **Root cause:** When building the list of valid columns for ORDER BY validation in `select.rs`, alias names were converted to `Column` using `.into()`, which calls `from_qualified_name()` and normalizes identifiers to lowercase. However, ORDER BY positional references resolve to columns using schema field names, which preserve case. This caused a mismatch (e.g., `Column("Count")` vs `Column("count")`). **Fix:** Use `Column::new_unqualified()` instead of `.into()` to preserve the exact case of alias names, matching how the schema stores field names. ## Are these changes tested? Yes, added a regression test to `order.slt`. ## Are there any user-facing changes? No, this is a bug fix that restores expected behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.5 <[email protected]>
1 parent 2e3707e commit b3d2cb6

File tree

2 files changed

+16
-3
lines changed

2 files changed

+16
-3
lines changed

datafusion/sql/src/select.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ use crate::utils::{
2929

3030
use datafusion_common::error::DataFusionErrorBuilder;
3131
use datafusion_common::tree_node::{TreeNode, TreeNodeRecursion};
32+
use datafusion_common::{Column, Result, not_impl_err, plan_err};
3233
use datafusion_common::{RecursionUnnestOption, UnnestOptions};
33-
use datafusion_common::{Result, not_impl_err, plan_err};
3434
use datafusion_expr::expr::{Alias, PlannedReplaceSelectItem, WildcardOptions};
3535
use datafusion_expr::expr_rewriter::{
3636
normalize_col, normalize_col_with_schemas_and_ambiguity_check, normalize_sorts,
@@ -1054,7 +1054,9 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
10541054
&& alias.expr.as_ref() == &rewritten_expr
10551055
{
10561056
// Use the alias name
1057-
return Some(Expr::Column(alias.name.clone().into()));
1057+
return Some(Expr::Column(Column::new_unqualified(
1058+
alias.name.clone(),
1059+
)));
10581060
}
10591061
None
10601062
})
@@ -1069,7 +1071,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
10691071
.cloned()
10701072
.chain(select_exprs_post_aggr.iter().filter_map(|e| {
10711073
if let Expr::Alias(alias) = e {
1072-
Some(Expr::Column(alias.name.clone().into()))
1074+
Some(Expr::Column(Column::new_unqualified(alias.name.clone())))
10731075
} else {
10741076
None
10751077
}

datafusion/sqllogictest/test_files/order.slt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,17 @@ select column1 from foo group by column1 order by min(column2), max(column2);
444444
3
445445
5
446446

447+
# Test GROUP BY alias with ORDER BY column index
448+
# Regression test: GROUP BY an aliased column, ORDER BY using column index
449+
query TI
450+
with t as (select 'foo' as x)
451+
select x, count(*) as "Count"
452+
from t
453+
group by x
454+
order by 2 desc;
455+
----
456+
foo 1
457+
447458
# Test issue: https://github.com/apache/datafusion/issues/11549
448459
query I
449460
select column1 from foo order by log(column2);

0 commit comments

Comments
 (0)