aljaz/turso
1
0
Fork 0
mirror of https://github.com/aljazceru/turso.git synced 2026-02-20 07:25:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge 'Support numeric column references in GROUP BY' from Jussi Saurio

Browse Source 
We already supported this for ORDER BY but not GROUP BY - again noticed
this when running against some clickbench queries

Closes #1008
This commit is contained in:
Pekka Enberg
2025-02-15 11:02:28 +02:00
parent 71b15ae4f1 eca196a54b
commit c53897cb6c
2 changed files with 38 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

56
core/translate/select.rs
View File

@@ -300,6 +300,7 @@ pub fn prepare_select_plan<'a>(
if let Some(mut group_by) = group_by {
for expr in group_by.exprs.iter_mut() {
replace_column_number_with_copy_of_column_expr(expr, &plan.result_columns)?;
bind_column_references(
expr,
&plan.table_references,
@@ -343,35 +344,21 @@ pub fn prepare_select_plan<'a>(
if let Some(order_by) = select.order_by {
let mut key = Vec::new();
for o in order_by {
// if the ORDER BY expression is a number, interpret it as an 1-indexed column number
// otherwise, interpret it normally as an expression
let mut expr = if let ast::Expr::Literal(ast::Literal::Numeric(num)) = &o.expr {
let column_number = num.parse::<usize>()?;
if column_number == 0 {
crate::bail_parse_error!("invalid column index: {}", column_number);
}
let maybe_result_column = columns.get(column_number - 1);
match maybe_result_column {
Some(ResultColumn::Expr(e, _)) => e.clone(),
None => {
crate::bail_parse_error!("invalid column index: {}", column_number)
}
_ => todo!(),
}
} else {
o.expr
};
for mut o in order_by {
replace_column_number_with_copy_of_column_expr(
&mut o.expr,
&plan.result_columns,
)?;
bind_column_references(
&mut expr,
&mut o.expr,
&plan.table_references,
Some(&plan.result_columns),
)?;
resolve_aggregates(&expr, &mut plan.aggregates);
resolve_aggregates(&o.expr, &mut plan.aggregates);
key.push((
expr,
o.expr,
o.order.map_or(Direction::Ascending, |o| match o {
ast::SortOrder::Asc => Direction::Ascending,
ast::SortOrder::Desc => Direction::Descending,
@@ -392,6 +379,31 @@ pub fn prepare_select_plan<'a>(
}
}
/// Replaces a column number in an ORDER BY or GROUP BY expression with a copy of the column expression.
/// For example, in SELECT u.first_name, count(1) FROM users u GROUP BY 1 ORDER BY 2,
/// the column number 1 is replaced with u.first_name and the column number 2 is replaced with count(1).
fn replace_column_number_with_copy_of_column_expr(
order_by_or_group_by_expr: &mut ast::Expr,
columns: &[ResultSetColumn],
) -> Result<()> {
if let ast::Expr::Literal(ast::Literal::Numeric(num)) = order_by_or_group_by_expr {
let column_number = num.parse::<usize>()?;
if column_number == 0 {
crate::bail_parse_error!("invalid column index: {}", column_number);
}
let maybe_result_column = columns.get(column_number - 1);
match maybe_result_column {
Some(ResultSetColumn { expr, .. }) => {
*order_by_or_group_by_expr = expr.clone();
}
None => {
crate::bail_parse_error!("invalid column index: {}", column_number)
}
};
}
Ok(())
}
fn count_plan_required_cursors(plan: &SelectPlan) -> usize {
let num_table_cursors: usize = plan
.table_references

4
testing/groupby.test
View File

@@ -181,3 +181,7 @@ do_execsql_test column_alias_in_group_by_order_by_having {
select first_name as fn, count(1) as fn_count from users where fn in ('Wanda', 'Whitney', 'William') group by fn having fn_count > 10 order by fn_count;
} {Whitney|11
William|111}
do_execsql_test group_by_column_number {
select u.first_name, count(1) from users u group by 1 limit 1;
} {Aaron|41}