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

fix: result columns have varying binding precedence

Browse Source 
In e.g. `SELECT x AS y, y AS x FROM t ORDER BY x;`, the `x` in the
`ORDER BY` should reference t.y, which has been aliased as `x` for this
query. The same goes for GROUP BY, JOIN ON etc. but NOT for WHERE.

Previously we had wrong precedence in `bind_and_rewrite_expr`.
This commit is contained in:
Jussi Saurio
2025-09-24 09:44:24 +03:00
parent 0a78ea87d5
commit c18c44b032
9 changed files with 145 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
core/schema.rs
View File

@@ -1,6 +1,8 @@
use crate::function::Func;
use crate::incremental::view::IncrementalView;
use crate::translate::expr::{bind_and_rewrite_expr, walk_expr, ParamState, WalkControl};
use crate::translate::expr::{
bind_and_rewrite_expr, walk_expr, BindingBehavior, ParamState, WalkControl,
};
use crate::translate::planner::ROWID_STRS;
use parking_lot::RwLock;
@@ -1852,7 +1854,15 @@ impl Index {
};
let mut params = ParamState::disallow();
let mut expr = where_clause.clone();
bind_and_rewrite_expr(&mut expr, table_refs, None, connection, &mut params).ok()?;
bind_and_rewrite_expr(
&mut expr,
table_refs,
None,
connection,
&mut params,
BindingBehavior::ResultColumnsNotAllowed,
)
.ok()?;
Some(*expr)
}
}

48
core/translate/expr.rs
View File

@@ -3273,6 +3273,20 @@ impl ParamState {
}
}
/// The precedence of binding identifiers to columns.
///
/// TryResultColumnsFirst means that result columns (e.g. SELECT x AS y, ...) take precedence over canonical columns (e.g. SELECT x, y AS z, ...). This is the default behavior.
///
/// TryCanonicalColumnsFirst means that canonical columns take precedence over result columns. This is used for e.g. WHERE clauses.
///
/// ResultColumnsNotAllowed means that referring to result columns is not allowed. This is used e.g. for DML statements.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum BindingBehavior {
TryResultColumnsFirst,
TryCanonicalColumnsFirst,
ResultColumnsNotAllowed,
}
/// Rewrite ast::Expr in place, binding Column references/rewriting Expr::Id -> Expr::Column
/// using the provided TableReferences, and replacing anonymous parameters with internal named
/// ones, as well as normalizing any DoublyQualified/Qualified quoted identifiers.
@@ -3282,6 +3296,7 @@ pub fn bind_and_rewrite_expr<'a>(
result_columns: Option<&'a [ResultSetColumn]>,
connection: &'a Arc<crate::Connection>,
param_state: &mut ParamState,
binding_behavior: BindingBehavior,
) -> Result<WalkControl> {
walk_expr_mut(
top_level_expr,
@@ -3340,9 +3355,21 @@ pub fn bind_and_rewrite_expr<'a>(
}
if let Some(referenced_tables) = &mut referenced_tables {
match expr {
// Unqualified identifier binding (including rowid aliases, outer refs, result-column fallback).
Expr::Id(id) => {
let normalized_id = normalize_ident(id.as_str());
if binding_behavior == BindingBehavior::TryResultColumnsFirst {
if let Some(result_columns) = result_columns {
for result_column in result_columns.iter() {
if result_column.name(referenced_tables).is_some_and(|name| {
name.eq_ignore_ascii_case(&normalized_id)
}) {
*expr = result_column.expr.clone();
return Ok(WalkControl::Continue);
}
}
}
}
if !referenced_tables.joined_tables().is_empty() {
if let Some(row_id_expr) = parse_row_id(
&normalized_id,
@@ -3421,17 +3448,19 @@ pub fn bind_and_rewrite_expr<'a>(
return Ok(WalkControl::Continue);
}
if let Some(result_columns) = result_columns {
for result_column in result_columns.iter() {
if result_column
.name(referenced_tables)
.is_some_and(|name| name.eq_ignore_ascii_case(&normalized_id))
{
*expr = result_column.expr.clone();
return Ok(WalkControl::Continue);
if binding_behavior == BindingBehavior::TryCanonicalColumnsFirst {
if let Some(result_columns) = result_columns {
for result_column in result_columns.iter() {
if result_column.name(referenced_tables).is_some_and(|name| {
name.eq_ignore_ascii_case(&normalized_id)
}) {
*expr = result_column.expr.clone();
return Ok(WalkControl::Continue);
}
}
}
}
// SQLite behavior: Only double-quoted identifiers get fallback to string literals
// Single quotes are handled as literals earlier, unquoted identifiers must resolve to columns
if id.is_double_quoted() {
@@ -4054,6 +4083,7 @@ pub fn process_returning_clause(
None,
connection,
param_ctx,
BindingBehavior::TryResultColumnsFirst,
)?;
result_columns.push(ResultSetColumn {

29
core/translate/insert.rs
View File

@@ -14,7 +14,7 @@ use crate::translate::emitter::{
};
use crate::translate::expr::{
bind_and_rewrite_expr, emit_returning_results, process_returning_clause, walk_expr_mut,
ParamState, ReturningValueRegisters, WalkControl,
BindingBehavior, ParamState, ReturningValueRegisters, WalkControl,
};
use crate::translate::plan::TableReferences;
use crate::translate::planner::ROWID_STRS;
@@ -148,7 +148,14 @@ pub fn translate_insert(
}
_ => {}
}
bind_and_rewrite_expr(expr, None, None, connection, &mut param_ctx)?;
bind_and_rewrite_expr(
expr,
None,
None,
connection,
&mut param_ctx,
BindingBehavior::ResultColumnsNotAllowed,
)?;
}
values = values_expr.pop();
}
@@ -161,10 +168,24 @@ pub fn translate_insert(
} = &mut upsert.do_clause
{
for set in sets.iter_mut() {
bind_and_rewrite_expr(&mut set.expr, None, None, connection, &mut param_ctx)?;
bind_and_rewrite_expr(
&mut set.expr,
None,
None,
connection,
&mut param_ctx,
BindingBehavior::ResultColumnsNotAllowed,
)?;
}
if let Some(ref mut where_expr) = where_clause {
bind_and_rewrite_expr(where_expr, None, None, connection, &mut param_ctx)?;
bind_and_rewrite_expr(
where_expr,
None,
None,
connection,
&mut param_ctx,
BindingBehavior::ResultColumnsNotAllowed,
)?;
}
}
}

22
core/translate/planner.rs
View File

@@ -11,7 +11,7 @@ use super::{
select::prepare_select_plan,
SymbolTable,
};
use crate::translate::expr::WalkControl;
use crate::translate::expr::{BindingBehavior, WalkControl};
use crate::translate::plan::{Window, WindowFunction};
use crate::{
ast::Limit,
@@ -686,6 +686,7 @@ pub fn parse_where(
result_columns,
connection,
param_ctx,
BindingBehavior::TryCanonicalColumnsFirst,
)?;
}
Ok(())
@@ -973,6 +974,7 @@ fn parse_join(
None,
connection,
param_ctx,
BindingBehavior::TryResultColumnsFirst,
)?;
}
}
@@ -1117,9 +1119,23 @@ pub fn parse_limit(
connection: &std::sync::Arc<crate::Connection>,
param_ctx: &mut ParamState,
) -> Result<(Option<Box<Expr>>, Option<Box<Expr>>)> {
bind_and_rewrite_expr(&mut limit.expr, None, None, connection, param_ctx)?;
bind_and_rewrite_expr(
&mut limit.expr,
None,
None,
connection,
param_ctx,
BindingBehavior::TryResultColumnsFirst,
)?;
if let Some(ref mut off_expr) = limit.offset {
bind_and_rewrite_expr(off_expr, None, None, connection, param_ctx)?;
bind_and_rewrite_expr(
off_expr,
None,
None,
connection,
param_ctx,
BindingBehavior::TryResultColumnsFirst,
)?;
}
Ok((Some(limit.expr.clone()), limit.offset.clone()))
}

9
core/translate/select.rs
View File

@@ -5,7 +5,7 @@ use super::plan::{
};
use crate::schema::Table;
use crate::translate::emitter::Resolver;
use crate::translate::expr::{bind_and_rewrite_expr, ParamState};
use crate::translate::expr::{bind_and_rewrite_expr, BindingBehavior, ParamState};
use crate::translate::group_by::compute_group_by_sort_order;
use crate::translate::optimizer::optimize_plan;
use crate::translate::plan::{GroupBy, Plan, ResultSetColumn, SelectPlan};
@@ -302,6 +302,7 @@ fn prepare_one_select_plan(
None,
connection,
param_ctx,
BindingBehavior::ResultColumnsNotAllowed,
)?;
}
for (expr, _) in window.order_by.iter_mut() {
@@ -311,6 +312,7 @@ fn prepare_one_select_plan(
None,
connection,
param_ctx,
BindingBehavior::ResultColumnsNotAllowed,
)?;
}
@@ -373,6 +375,7 @@ fn prepare_one_select_plan(
None,
connection,
param_ctx,
BindingBehavior::ResultColumnsNotAllowed,
)?;
let contains_aggregates = resolve_window_and_aggregate_functions(
schema,
@@ -422,6 +425,7 @@ fn prepare_one_select_plan(
Some(&plan.result_columns),
connection,
param_ctx,
BindingBehavior::TryResultColumnsFirst,
)?;
}
@@ -438,6 +442,7 @@ fn prepare_one_select_plan(
Some(&plan.result_columns),
connection,
param_ctx,
BindingBehavior::TryResultColumnsFirst,
)?;
let contains_aggregates = resolve_window_and_aggregate_functions(
schema,
@@ -477,6 +482,7 @@ fn prepare_one_select_plan(
Some(&plan.result_columns),
connection,
param_ctx,
BindingBehavior::TryResultColumnsFirst,
)?;
resolve_window_and_aggregate_functions(
schema,
@@ -561,6 +567,7 @@ fn add_vtab_predicates_to_where_clause(
Some(&plan.result_columns),
connection,
param_ctx,
BindingBehavior::TryCanonicalColumnsFirst,
)?;
}
for expr in vtab_predicates.drain(..) {

7
core/translate/update.rs
View File

@@ -2,7 +2,9 @@ use std::collections::{HashMap, HashSet};
use std::sync::Arc;
use crate::schema::{BTreeTable, Column, Type};
use crate::translate::expr::{bind_and_rewrite_expr, walk_expr, ParamState, WalkControl};
use crate::translate::expr::{
bind_and_rewrite_expr, walk_expr, BindingBehavior, ParamState, WalkControl,
};
use crate::translate::optimizer::optimize_select_plan;
use crate::translate::plan::{Operation, QueryDestination, Scan, Search, SelectPlan};
use crate::translate::planner::parse_limit;
@@ -191,6 +193,7 @@ pub fn prepare_update_plan(
None,
connection,
&mut param_idx,
BindingBehavior::ResultColumnsNotAllowed,
)?;
let values = match set.expr.as_ref() {
@@ -241,6 +244,7 @@ pub fn prepare_update_plan(
Some(&result_columns),
connection,
&mut param_idx,
BindingBehavior::ResultColumnsNotAllowed,
);
(o.expr.clone(), o.order.unwrap_or(SortOrder::Asc))
})
@@ -400,6 +404,7 @@ pub fn prepare_update_plan(
None,
connection,
&mut param,
BindingBehavior::ResultColumnsNotAllowed,
)
.ok()?;
let cols_used = collect_cols_used_in_expr(&where_copy);

8
testing/groupby.test
View File

@@ -346,3 +346,11 @@ Daniel|1
Cindy|1
Aimee|1}
# In GROUP BY clauses, column aliases take precedence when resolving identifiers to columns.
do_execsql_test_on_specific_db {:memory:} group_by_alias_precedence {
CREATE TABLE t(x,y);
INSERT INTO t VALUES (1,200),(2,100);
INSERT INTO t VALUES (1,200),(2,100);
SELECT x AS y, SUM(y) as x FROM t GROUP BY y ORDER BY x;
} {2|200
1|400}

8
testing/orderby.test
View File

@@ -232,3 +232,11 @@ shorts|shorts|70.0
sneakers|sneakers|82.0
sweater|sweater|25.0
sweatshirt|sweatshirt|74.0}
# In ORDER BY clauses, column aliases take precedence when resolving identifiers to columns.
do_execsql_test_on_specific_db {:memory:} orderby_alias_precedence {
CREATE TABLE t(x,y);
INSERT INTO t VALUES (1,200),(2,100);
SELECT x AS y, y AS x FROM t ORDER BY x;
} {2|100
1|200}

20
testing/where.test
View File

@@ -590,3 +590,23 @@ do_execsql_test where-null-comparison-index-seek-regression-test {
do_execsql_test where-self-referential-regression {
select count(1) from users where id = id;
} {10000}
# in WHERE clauses, column aliases do not take precedence when resolving identifiers to columns.
do_execsql_test_on_specific_db {:memory:} where_alias_precedence {
CREATE TABLE t(x,y);
INSERT INTO t VALUES (1,200),(2,100);
SELECT x AS y, y AS x FROM t WHERE x = 100;
} {}
# More aliasing tests
do_execsql_test_on_specific_db {:memory:} where_alias_precedence_2 {
CREATE TABLE t(x,y);
INSERT INTO t SELECT value, value+100 FROM generate_series(1,3);
SELECT x AS lol, y AS x FROM t WHERE x = 101;
} {}
do_execsql_test_on_specific_db {:memory:} where_alias_precedence_3 {
CREATE TABLE t(x,y);
INSERT INTO t SELECT value, value+100 FROM generate_series(1,3);
SELECT x AS lol, y AS lmao FROM t WHERE lmao = 101;
} {1|101}