From a012e98bfae70104445e3da41cbd2ffec9775c85 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Fri, 7 Nov 2025 19:18:10 -0500 Subject: [PATCH 1/2] core/translate remove unused ParamState and some minor refactoring --- core/schema.rs | 6 +----- core/translate/alter.rs | 10 ++++------ core/translate/delete.rs | 6 ++---- core/translate/expr.rs | 29 ----------------------------- core/translate/insert.rs | 3 --- core/translate/mod.rs | 4 +--- core/translate/planner.rs | 12 +++--------- core/translate/select.rs | 32 +++++++------------------------- core/translate/update.rs | 21 +++++++-------------- core/vdbe/builder.rs | 3 --- 10 files changed, 25 insertions(+), 101 deletions(-) diff --git a/core/schema.rs b/core/schema.rs index 73d06ea3c..3d80649c9 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -1,9 +1,7 @@ use crate::function::Func; use crate::incremental::view::IncrementalView; use crate::index_method::{IndexMethodAttachment, IndexMethodConfiguration}; -use crate::translate::expr::{ - bind_and_rewrite_expr, walk_expr, BindingBehavior, ParamState, WalkControl, -}; +use crate::translate::expr::{bind_and_rewrite_expr, walk_expr, BindingBehavior, WalkControl}; use crate::translate::index::{resolve_index_method_parameters, resolve_sorted_columns}; use crate::translate::planner::ROWID_STRS; use parking_lot::RwLock; @@ -2835,14 +2833,12 @@ impl Index { let Some(where_clause) = &self.where_clause else { return None; }; - let mut params = ParamState::disallow(); let mut expr = where_clause.clone(); bind_and_rewrite_expr( &mut expr, table_refs, None, connection, - &mut params, BindingBehavior::ResultColumnsNotAllowed, ) .ok()?; diff --git a/core/translate/alter.rs b/core/translate/alter.rs index 4a4d6664e..0ce64969e 100644 --- a/core/translate/alter.rs +++ b/core/translate/alter.rs @@ -202,13 +202,12 @@ pub fn translate_alter_table( ); let mut parser = Parser::new(stmt.as_bytes()); - let Some(ast::Cmd::Stmt(ast::Stmt::Update(mut update))) = parser.next_cmd().unwrap() - else { + let Some(ast::Cmd::Stmt(ast::Stmt::Update(update))) = parser.next_cmd().unwrap() else { unreachable!(); }; translate_update_for_schema_change( - &mut update, + update, resolver, program, connection, @@ -330,13 +329,12 @@ pub fn translate_alter_table( ); let mut parser = Parser::new(stmt.as_bytes()); - let Some(ast::Cmd::Stmt(ast::Stmt::Update(mut update))) = parser.next_cmd().unwrap() - else { + let Some(ast::Cmd::Stmt(ast::Stmt::Update(update))) = parser.next_cmd().unwrap() else { unreachable!(); }; translate_update_for_schema_change( - &mut update, + update, resolver, program, connection, diff --git a/core/translate/delete.rs b/core/translate/delete.rs index e9d49da49..0ca50dd74 100644 --- a/core/translate/delete.rs +++ b/core/translate/delete.rs @@ -129,13 +129,11 @@ pub fn prepare_delete_plan( None, &mut where_predicates, connection, - &mut program.param_ctx, )?; // Parse the LIMIT/OFFSET clause - let (resolved_limit, resolved_offset) = limit.map_or(Ok((None, None)), |mut l| { - parse_limit(&mut l, connection, &mut program.param_ctx) - })?; + let (resolved_limit, resolved_offset) = + limit.map_or(Ok((None, None)), |l| parse_limit(l, connection))?; let plan = DeletePlan { table_references, diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 142c11ccc..56142508b 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -3563,28 +3563,6 @@ where Ok(WalkControl::Continue) } -pub struct ParamState { - // flag which allow or forbid usage of parameters during translation of AST to the program - // - // for example, parameters are not allowed in the partial index definition - // so tursodb set allowed to false when it parsed WHERE clause of partial index definition - pub allowed: bool, -} - -impl Default for ParamState { - fn default() -> Self { - Self { allowed: true } - } -} -impl ParamState { - pub fn is_valid(&self) -> bool { - self.allowed - } - pub fn disallow() -> Self { - Self { allowed: false } - } -} - /// 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. @@ -3610,18 +3588,12 @@ pub fn bind_and_rewrite_expr<'a>( mut referenced_tables: Option<&'a mut TableReferences>, result_columns: Option<&'a [ResultSetColumn]>, connection: &'a Arc, - param_state: &mut ParamState, binding_behavior: BindingBehavior, ) -> Result<()> { walk_expr_mut( top_level_expr, &mut |expr: &mut ast::Expr| -> Result { match expr { - ast::Expr::Variable(_) => { - if !param_state.is_valid() { - crate::bail_parse_error!("Parameters are not allowed in this context"); - } - } ast::Expr::Between { lhs, not, @@ -4422,7 +4394,6 @@ pub fn process_returning_clause( Some(&mut table_references), None, connection, - &mut program.param_ctx, BindingBehavior::TryResultColumnsFirst, )?; diff --git a/core/translate/insert.rs b/core/translate/insert.rs index f4e20669a..2082996b0 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -959,7 +959,6 @@ fn bind_insert( None, None, connection, - &mut program.param_ctx, BindingBehavior::ResultColumnsNotAllowed, )?; } @@ -1003,7 +1002,6 @@ fn bind_insert( None, None, connection, - &mut program.param_ctx, BindingBehavior::AllowUnboundIdentifiers, )?; } @@ -1013,7 +1011,6 @@ fn bind_insert( None, None, connection, - &mut program.param_ctx, BindingBehavior::AllowUnboundIdentifiers, )?; } diff --git a/core/translate/mod.rs b/core/translate/mod.rs index 6a36baef6..5d9b902ae 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -259,9 +259,7 @@ pub fn translate_inner( )? .program } - ast::Stmt::Update(mut update) => { - translate_update(&mut update, resolver, program, connection)? - } + ast::Stmt::Update(update) => translate_update(update, resolver, program, connection)?, ast::Stmt::Vacuum { .. } => bail_parse_error!("VACUUM not supported yet"), ast::Stmt::Insert { with, diff --git a/core/translate/planner.rs b/core/translate/planner.rs index ed1a27d8d..5a10e27cb 100644 --- a/core/translate/planner.rs +++ b/core/translate/planner.rs @@ -24,7 +24,7 @@ use crate::{ }; use crate::{ function::{AggFunc, ExtFunc}, - translate::expr::{bind_and_rewrite_expr, ParamState}, + translate::expr::bind_and_rewrite_expr, }; use crate::{ translate::plan::{Window, WindowFunction}, @@ -755,7 +755,6 @@ pub fn parse_where( result_columns: Option<&[ResultSetColumn]>, out_where_clause: &mut Vec, connection: &Arc, - param_ctx: &mut ParamState, ) -> Result<()> { if let Some(where_expr) = where_clause { let start_idx = out_where_clause.len(); @@ -766,7 +765,6 @@ pub fn parse_where( Some(table_references), result_columns, connection, - param_ctx, BindingBehavior::TryCanonicalColumnsFirst, )?; } @@ -1135,7 +1133,6 @@ fn parse_join( Some(table_references), None, connection, - &mut program.param_ctx, BindingBehavior::TryResultColumnsFirst, )?; } @@ -1277,16 +1274,14 @@ where #[allow(clippy::type_complexity)] pub fn parse_limit( - limit: &mut Limit, + mut limit: Limit, connection: &std::sync::Arc, - param_ctx: &mut ParamState, ) -> Result<(Option>, Option>)> { bind_and_rewrite_expr( &mut limit.expr, None, None, connection, - param_ctx, BindingBehavior::TryResultColumnsFirst, )?; if let Some(ref mut off_expr) = limit.offset { @@ -1295,9 +1290,8 @@ pub fn parse_limit( None, None, connection, - param_ctx, BindingBehavior::TryResultColumnsFirst, )?; } - Ok((Some(limit.expr.clone()), limit.offset.clone())) + Ok((Some(limit.expr), limit.offset)) } diff --git a/core/translate/select.rs b/core/translate/select.rs index 9ce153ff4..19e343e87 100644 --- a/core/translate/select.rs +++ b/core/translate/select.rs @@ -5,9 +5,7 @@ use super::plan::{ }; use crate::schema::Table; use crate::translate::emitter::{OperationMode, Resolver}; -use crate::translate::expr::{ - bind_and_rewrite_expr, expr_vector_size, BindingBehavior, ParamState, -}; +use crate::translate::expr::{bind_and_rewrite_expr, expr_vector_size, BindingBehavior}; use crate::translate::group_by::compute_group_by_sort_order; use crate::translate::optimizer::optimize_plan; use crate::translate::plan::{GroupBy, Plan, ResultSetColumn, SelectPlan}; @@ -149,9 +147,9 @@ pub fn prepare_select_plan( crate::bail_parse_error!("SELECTs to the left and right of {} do not have the same number of result columns", operator); } } - let (limit, offset) = select.limit.map_or(Ok((None, None)), |mut l| { - parse_limit(&mut l, connection, &mut program.param_ctx) - })?; + let (limit, offset) = select + .limit + .map_or(Ok((None, None)), |l| parse_limit(l, connection))?; // FIXME: handle ORDER BY for compound selects if !select.order_by.is_empty() { @@ -290,7 +288,6 @@ fn prepare_one_select_plan( Some(&mut plan.table_references), None, connection, - &mut program.param_ctx, BindingBehavior::ResultColumnsNotAllowed, )?; } @@ -300,7 +297,6 @@ fn prepare_one_select_plan( Some(&mut plan.table_references), None, connection, - &mut program.param_ctx, BindingBehavior::ResultColumnsNotAllowed, )?; } @@ -363,7 +359,6 @@ fn prepare_one_select_plan( Some(&mut plan.table_references), None, connection, - &mut program.param_ctx, BindingBehavior::ResultColumnsNotAllowed, )?; let contains_aggregates = resolve_window_and_aggregate_functions( @@ -387,12 +382,7 @@ fn prepare_one_select_plan( // This step can only be performed at this point, because all table references are now available. // Virtual table predicates may depend on column bindings from tables to the right in the join order, // so we must wait until the full set of references has been collected. - add_vtab_predicates_to_where_clause( - &mut vtab_predicates, - &mut plan, - connection, - &mut program.param_ctx, - )?; + add_vtab_predicates_to_where_clause(&mut vtab_predicates, &mut plan, connection)?; // Parse the actual WHERE clause and add its conditions to the plan WHERE clause that already contains the join conditions. parse_where( @@ -401,7 +391,6 @@ fn prepare_one_select_plan( Some(&plan.result_columns), &mut plan.where_clause, connection, - &mut program.param_ctx, )?; if let Some(mut group_by) = group_by { @@ -412,7 +401,6 @@ fn prepare_one_select_plan( Some(&mut plan.table_references), Some(&plan.result_columns), connection, - &mut program.param_ctx, BindingBehavior::TryResultColumnsFirst, )?; } @@ -429,7 +417,6 @@ fn prepare_one_select_plan( Some(&mut plan.table_references), Some(&plan.result_columns), connection, - &mut program.param_ctx, BindingBehavior::TryResultColumnsFirst, )?; let contains_aggregates = resolve_window_and_aggregate_functions( @@ -468,7 +455,6 @@ fn prepare_one_select_plan( Some(&mut plan.table_references), Some(&plan.result_columns), connection, - &mut program.param_ctx, BindingBehavior::TryResultColumnsFirst, )?; resolve_window_and_aggregate_functions( @@ -493,9 +479,8 @@ fn prepare_one_select_plan( } // Parse the LIMIT/OFFSET clause - (plan.limit, plan.offset) = limit.map_or(Ok((None, None)), |mut l| { - parse_limit(&mut l, connection, &mut program.param_ctx) - })?; + (plan.limit, plan.offset) = + limit.map_or(Ok((None, None)), |l| parse_limit(l, connection))?; if !windows.is_empty() { plan_windows( @@ -538,7 +523,6 @@ fn prepare_one_select_plan( None, None, connection, - &mut program.param_ctx, // Allow sqlite quirk of inserting "double-quoted" literals (which our AST maps as identifiers) BindingBehavior::AllowUnboundIdentifiers, )?; @@ -657,7 +641,6 @@ fn add_vtab_predicates_to_where_clause( vtab_predicates: &mut Vec, plan: &mut SelectPlan, connection: &Arc, - param_ctx: &mut ParamState, ) -> Result<()> { for expr in vtab_predicates.iter_mut() { bind_and_rewrite_expr( @@ -665,7 +648,6 @@ fn add_vtab_predicates_to_where_clause( Some(&mut plan.table_references), Some(&plan.result_columns), connection, - param_ctx, BindingBehavior::TryCanonicalColumnsFirst, )?; } diff --git a/core/translate/update.rs b/core/translate/update.rs index f542594ab..635e41051 100644 --- a/core/translate/update.rs +++ b/core/translate/update.rs @@ -3,9 +3,7 @@ use std::sync::Arc; use crate::schema::ROWID_SENTINEL; use crate::translate::emitter::Resolver; -use crate::translate::expr::{ - bind_and_rewrite_expr, walk_expr, BindingBehavior, ParamState, WalkControl, -}; +use crate::translate::expr::{bind_and_rewrite_expr, walk_expr, BindingBehavior, WalkControl}; use crate::translate::plan::{Operation, Scan}; use crate::translate::planner::{parse_limit, ROWID_STRS}; use crate::{ @@ -53,7 +51,7 @@ addr opcode p1 p2 p3 p4 p5 comment 18 Goto 0 1 0 0 */ pub fn translate_update( - body: &mut ast::Update, + body: ast::Update, resolver: &Resolver, mut program: ProgramBuilder, connection: &Arc, @@ -71,7 +69,7 @@ pub fn translate_update( } pub fn translate_update_for_schema_change( - body: &mut ast::Update, + body: ast::Update, resolver: &Resolver, mut program: ProgramBuilder, connection: &Arc, @@ -100,7 +98,7 @@ pub fn translate_update_for_schema_change( pub fn prepare_update_plan( program: &mut ProgramBuilder, schema: &Schema, - body: &mut ast::Update, + mut body: ast::Update, connection: &Arc, is_internal_schema_change: bool, ) -> crate::Result { @@ -211,7 +209,6 @@ pub fn prepare_update_plan( Some(&mut table_references), None, connection, - &mut program.param_ctx, BindingBehavior::ResultColumnsNotAllowed, )?; @@ -286,7 +283,6 @@ pub fn prepare_update_plan( Some(&mut table_references), Some(&result_columns), connection, - &mut program.param_ctx, BindingBehavior::ResultColumnsNotAllowed, ); (o.expr.clone(), o.order.unwrap_or(SortOrder::Asc)) @@ -306,13 +302,12 @@ pub fn prepare_update_plan( Some(&result_columns), &mut where_clause, connection, - &mut program.param_ctx, )?; // Parse the LIMIT/OFFSET clause - let (limit, offset) = body.limit.as_mut().map_or(Ok((None, None)), |l| { - parse_limit(l, connection, &mut program.param_ctx) - })?; + let (limit, offset) = body + .limit + .map_or(Ok((None, None)), |l| parse_limit(l, connection))?; // Check what indexes will need to be updated by checking set_clauses and see // if a column is contained in an index. @@ -337,7 +332,6 @@ pub fn prepare_update_plan( if !needs { if let Some(w) = &idx.where_clause { let mut where_copy = w.as_ref().clone(); - let mut param = ParamState::disallow(); let mut tr = TableReferences::new(table_references.joined_tables().to_vec(), vec![]); bind_and_rewrite_expr( @@ -345,7 +339,6 @@ pub fn prepare_update_plan( Some(&mut tr), None, connection, - &mut param, BindingBehavior::ResultColumnsNotAllowed, ) .ok()?; diff --git a/core/vdbe/builder.rs b/core/vdbe/builder.rs index 05a89cfff..8b522dfea 100644 --- a/core/vdbe/builder.rs +++ b/core/vdbe/builder.rs @@ -14,7 +14,6 @@ use crate::{ translate::{ collate::CollationSeq, emitter::TransactionMode, - expr::ParamState, plan::{ResultSetColumn, TableReferences}, }, CaptureDataChangesMode, Connection, Value, VirtualTable, @@ -123,7 +122,6 @@ pub struct ProgramBuilder { query_mode: QueryMode, /// Current parent explain address, if any. current_parent_explain_idx: Option, - pub param_ctx: ParamState, pub(crate) reg_result_cols_start: Option, /// Whether the program needs to use statement subtransactions, /// i.e. the individual statement may need to be aborted due to a constraint conflict, etc. @@ -215,7 +213,6 @@ impl ProgramBuilder { rollback: false, query_mode, current_parent_explain_idx: None, - param_ctx: ParamState::default(), reg_result_cols_start: None, needs_stmt_subtransactions: false, } From dd2e3e8e16ac673234adddead10bc9c63ecfc8c6 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Fri, 7 Nov 2025 20:04:57 -0500 Subject: [PATCH 2/2] Fix clippy warning --- core/translate/expr.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 56142508b..5bd53603a 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -3619,9 +3619,6 @@ pub fn bind_and_rewrite_expr<'a>( ast::Expr::Binary(Box::new(lower), ast::Operator::And, Box::new(upper)) }; } - _ => {} - } - match expr { Expr::Id(id) => { let Some(referenced_tables) = &mut referenced_tables else { if binding_behavior == BindingBehavior::AllowUnboundIdentifiers {