diff --git a/core/parameters.rs b/core/parameters.rs index b4070a7c9..a3dab16c8 100644 --- a/core/parameters.rs +++ b/core/parameters.rs @@ -1,3 +1,4 @@ +use super::ast; use std::num::NonZero; #[derive(Clone, Debug)] @@ -23,13 +24,52 @@ impl Parameter { } } +#[derive(Debug)] +struct InsertContext { + param_positions: Vec, + current_col_value_idx: usize, +} + +impl InsertContext { + fn new(param_positions: Vec) -> Self { + Self { + param_positions, + current_col_value_idx: 0, + } + } + + /// Find the relevant parameter index needed for the current value index of insert stmt + /// Example for table t (a,b,c): + /// `insert into t (c,a,b) values (?,?,?)` + /// + /// col a -> value_index 1 + /// col b -> value_index 2 + /// col c -> value_index 0 + /// + /// however translation will always result in parameters 1, 2, 3 + /// because columns are translated in the table order so `col a` gets + /// translated first, translate_expr calls parameters.push and always gets index 1. + /// + /// Instead, we created an array representing all the value_index's that are type + /// Expr::Variable, in the case above would be [1, 2, 0], and stored it in insert_ctx. + /// That array can be used to look up the necessary parameter index by searching for the value + /// index in the array and returning the index of that value + 1. + /// value_index-> [1, 2, 0] + /// param index-> |0, 1, 2| + fn get_insert_param_index(&self) -> Option> { + self.param_positions + .iter() + .position(|param| param.eq(&self.current_col_value_idx)) + .map(|p| NonZero::new(p + 1).unwrap()) + } +} + #[derive(Debug)] pub struct Parameters { index: NonZero, pub list: Vec, - // Indexes of the referenced insert values to maintain ordering of paramaters - param_positions: Option>, - current_col_value_idx: Option, + // Context for reordering parameters during insert statements + insert_ctx: Option, } impl Default for Parameters { @@ -43,8 +83,7 @@ impl Parameters { Self { index: 1.try_into().unwrap(), list: vec![], - param_positions: None, - current_col_value_idx: None, + insert_ctx: None, } } @@ -54,24 +93,16 @@ impl Parameters { params.len() } - pub fn set_value_index(&mut self, idx: usize) { - self.current_col_value_idx = Some(idx); + /// Begin preparing for an Insert statement by providing the array of values from the Insert body. + pub fn init_insert_parameters(&mut self, values: &[Vec]) { + self.insert_ctx = Some(InsertContext::new(expected_param_indicies(values))); } - pub fn set_parameter_positions(&mut self, params: Vec) { - self.param_positions = Some(params); - } - - pub fn get_param_index(&self) -> Option> { - if let Some(val) = self.current_col_value_idx { - return self.param_positions.as_ref().and_then(|positions| { - positions - .iter() - .position(|param| param.eq(&val)) - .map(|p| NonZero::new(p + 1).unwrap()) - }); - }; - None + /// Set the value index for the column currently being translated for an Insert stmt. + pub fn set_insert_value_index(&mut self, idx: usize) { + if let Some(ctx) = &mut self.insert_ctx { + ctx.current_col_value_idx = idx; + } } pub fn name(&self, index: NonZero) -> Option { @@ -105,8 +136,8 @@ impl Parameters { let index = self.next_index(); self.list.push(Parameter::Anonymous(index)); tracing::trace!("anonymous parameter at {index}"); - if let Some(idx) = self.get_param_index() { - idx + if let Some(idx) = &self.insert_ctx { + idx.get_insert_param_index().unwrap_or(index) } else { index } @@ -144,3 +175,14 @@ impl Parameters { } } } + +/// Gather all the expected indicies of all Expr::Variable +/// in the provided array of insert values. +pub fn expected_param_indicies(cols: &[Vec]) -> Vec { + cols.iter() + .flat_map(|col| col.iter()) + .enumerate() + .filter(|(_, col)| matches!(col, ast::Expr::Variable(_))) + .map(|(i, _)| i) + .collect::>() +} diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 9d97c6cf7..629d2e40f 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -2538,16 +2538,6 @@ pub fn maybe_apply_affinity(col_type: Type, target_register: usize, program: &mu } } -/// Gather all the expected indicies of all Expr::Variable -pub fn expected_param_indicies(cols: &[Vec]) -> Vec { - cols.iter() - .flat_map(|col| col.iter()) - .enumerate() - .filter(|(_, col)| matches!(col, ast::Expr::Variable(_))) - .map(|(i, _)| i) - .collect::>() -} - /// Sanitaizes a string literal by removing single quote at front and back /// and escaping double single quotes pub fn sanitize_string(input: &str) -> String { diff --git a/core/translate/insert.rs b/core/translate/insert.rs index 141468121..233d623c9 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -21,7 +21,7 @@ use crate::{ use crate::{Result, SymbolTable, VirtualTable}; use super::emitter::Resolver; -use super::expr::{expected_param_indicies, translate_expr_no_constant_opt, NoConstantOptReason}; +use super::expr::{translate_expr_no_constant_opt, NoConstantOptReason}; #[allow(clippy::too_many_arguments)] pub fn translate_insert( @@ -106,11 +106,8 @@ pub fn translate_insert( }, InsertBody::DefaultValues => &vec![vec![]], }; - // set expected parameter value_indexes, so we can keep track of which value_index - // maps to each index of a variable in translate_expr - program - .parameters - .set_parameter_positions(expected_param_indicies(values)); + // prepare parameters by tracking the number of variables we will be binding to values later on + program.parameters.init_insert_parameters(values); let column_mappings = resolve_columns_for_insert(&table, columns, values)?; let index_col_mappings = resolve_indicies_for_insert(schema, table.as_ref(), &column_mappings)?; @@ -587,6 +584,7 @@ fn resolve_indicies_for_insert( } /// Populates the column registers with values for a single row +#[allow(clippy::too_many_arguments)] fn populate_column_registers( row_idx: usize, program: &mut ProgramBuilder, @@ -612,11 +610,14 @@ fn populate_column_registers( } else { target_reg }; - // for setting parameter value index, for: values (?,?), (?,?) - // value_index here needs to be (1,2),(3,4) instead of (1,2),(1,2) + // We need the 'parameters' to be aware of the value_index of the current row + // so it can map it to the correct parameter index in the Variable opcode + // but we need to make sure the value_index is not overwritten if this is a multi-row + // insert. For 'insert into t values: (?,?), (?,?);' + // value_index should be (1,2),(3,4) instead of (1,2),(1,2), so multiply by col length program .parameters - .set_value_index(value_index + (column_mappings.len() * row_idx)); + .set_insert_value_index(value_index + (column_mappings.len() * row_idx)); translate_expr_no_constant_opt( program, None, @@ -680,9 +681,8 @@ fn translate_virtual_table_insert( InsertBody::DefaultValues => &vec![], _ => crate::bail_parse_error!("Unsupported INSERT body for virtual tables"), }; - program - .parameters - .set_parameter_positions(expected_param_indicies(values)); + // initiate parameters by tracking the number of variables we will be binding to values + program.parameters.init_insert_parameters(values); let table = Table::Virtual(virtual_table.clone()); let column_mappings = resolve_columns_for_insert(&table, columns, values)?; let registers_start = program.alloc_registers(2);