diff --git a/doc/sql/query.ebnf b/doc/sql/query.ebnf index ab48e3cf2f93635030ad4e2622982ef1ade15ff5..7456d29ff642ebba0b5fbf815eee6ea923f0fbe5 100644 --- a/doc/sql/query.ebnf +++ b/doc/sql/query.ebnf @@ -47,7 +47,7 @@ reference ::= (table '.')? name value ::= 'TRUE' | 'FALSE' | 'NULL' - | '?' + | parameter | integer | unsigned | double @@ -67,6 +67,7 @@ type ::= 'ANY' | 'TEXT' | 'UNSIGNED' | 'VARCHAR' ('(' length ')')? +parameter ::= '$' unsigned | '?' delete ::= 'DELETE' 'FROM' table ('WHERE' expression)? insert ::= 'INSERT' 'INTO' table ('(' name (',' name)* ')')? (values row | select ) ('ON CONFLICT' 'DO' ('NOTHING' | 'REPLACE' | 'FAIL'))? update ::= 'UPDATE' table 'SET' ( name '=' ((table '.')? name) (',' name '=' (table '.')? name)* ) ( 'FROM' '(' select ')' ( 'AS' name )? )? ( 'WHERE' expression )? diff --git a/sbroad-cartridge/test_app/test/integration/api_test.lua b/sbroad-cartridge/test_app/test/integration/api_test.lua index c642e1ef944d4ae65ce42d24274a3f13790b5081..6d132759be8f4e381daeec1bccc610386c42171d 100644 --- a/sbroad-cartridge/test_app/test/integration/api_test.lua +++ b/sbroad-cartridge/test_app/test/integration/api_test.lua @@ -483,3 +483,47 @@ g.test_uppercase3 = function() rows = {}, }) end + +g.test_pg_style_params1 = function() + local api = cluster:server("api-1").net_box + + local r, err = api:call("sbroad.execute", { [[ + SELECT "id" FROM "t" + where "id" = $1 or "id" = $1 + 1 + ]], {1} }) + + t.assert_equals(err, nil) + t.assert_equals(r, { + metadata = { + {name = "id", type = "integer"}, + }, + rows = { + {1}, + {2} + }, + }) +end + +g.test_pg_style_params2 = function() + local api = cluster:server("api-1").net_box + + local r, err = api:call("sbroad.execute", { [[ + SELECT "id" + $1 as "id" from ( + SELECT "id" FROM "t" + UNION ALL + SELECT "id" FROM "space_simple_shard_key" + ) + group by "id" + $1 + having count("id") > $1 + ]], {1} }) + + t.assert_equals(err, nil) + t.assert_equals(r, { + metadata = { + {name = "id", type = "integer"}, + }, + rows = { + {2}, + }, + }) +end diff --git a/sbroad-core/src/errors.rs b/sbroad-core/src/errors.rs index 345ef62aee4332ee0056d1cd54cce62c757bb05f..da6b1472afe0abb0fffcec1d32eca8703d815a37 100644 --- a/sbroad-core/src/errors.rs +++ b/sbroad-core/src/errors.rs @@ -303,6 +303,7 @@ pub enum SbroadError { /// and can be empty (None). Unsupported(Entity, Option<String>), OutdatedStorageSchema, + UseOfBothParamsStyles, UnsupportedOpForGlobalTables(String), } @@ -328,6 +329,9 @@ impl fmt::Display for SbroadError { }, SbroadError::UnexpectedNumberOfValues(s) => format!("unexpected number of values: {s}"), SbroadError::LuaError(e) => e.clone(), + SbroadError::UseOfBothParamsStyles => { + "invalid parameters usage. Got $n and ? parameters in one query!".into() + } SbroadError::OutdatedStorageSchema => { "storage schema version different from router".into() } diff --git a/sbroad-core/src/executor/ir.rs b/sbroad-core/src/executor/ir.rs index 7ffa4eca1f1e2bd96067d3584d88e7e71cd2e4cd..c6cd6a3c93c95a039e2441871615ff435eb3f036 100644 --- a/sbroad-core/src/executor/ir.rs +++ b/sbroad-core/src/executor/ir.rs @@ -415,11 +415,12 @@ impl ExecutionPlan { } let dst_node = self.get_mut_ir_plan().get_mut_node(node_id)?; + let next_id = new_plan.nodes.next_id(); + // Replace the node with some invalid value. // TODO: introduce some new enum variant for this purpose. let mut node: Node = std::mem::replace(dst_node, Node::Parameter); let ir_plan = self.get_ir_plan(); - let next_id = new_plan.nodes.next_id(); match node { Node::Relational(ref mut rel) => { if let Relational::ValuesRow { data, .. } = rel { diff --git a/sbroad-core/src/executor/tests/exec_plan.rs b/sbroad-core/src/executor/tests/exec_plan.rs index e5c1186a8ffc17699639c0736b671378bbf84032..47b08897771df8989a2579d2a24d5d478cb36823 100644 --- a/sbroad-core/src/executor/tests/exec_plan.rs +++ b/sbroad-core/src/executor/tests/exec_plan.rs @@ -1154,3 +1154,65 @@ fn global_except() { ]]); assert_eq!(expected, res,) } + +fn check_subtree_hashes_are_equal( + sql1: &str, + values1: Vec<Value>, + sql2: &str, + values2: Vec<Value>, +) { + let coordinator = RouterRuntimeMock::new(); + let get_hash = |sql: &str, values: Vec<Value>| -> String { + let mut query = Query::new(&coordinator, sql, values).unwrap(); + query + .get_mut_exec_plan() + .get_mut_ir_plan() + .stash_constants() + .unwrap(); + let ir = query.get_exec_plan().get_ir_plan(); + let top = ir.get_top().unwrap(); + ir.pattern_id(top).unwrap() + }; + + assert_eq!(get_hash(sql1, values1), get_hash(sql2, values2)); +} + +#[test] +fn subtree_hash1() { + check_subtree_hashes_are_equal( + r#"select ?, ? from "t""#, + vec![Value::Unsigned(1), Value::Unsigned(1)], + r#"select $1, $1 from "t""#, + vec![Value::Unsigned(1)], + ); +} + +#[test] +fn subtree_hash2() { + check_subtree_hashes_are_equal( + r#"select ?, ? from "t" + option(sql_vdbe_max_steps = ?, vtable_max_rows = ?)"#, + vec![ + Value::Unsigned(1), + Value::Unsigned(11), + Value::Unsigned(3), + Value::Unsigned(10), + ], + r#"select $1, $1 from "t" + option(sql_vdbe_max_steps = $1, vtable_max_rows = $1)"#, + vec![Value::Unsigned(1)], + ); +} + +/* FIXME: https://git.picodata.io/picodata/picodata/sbroad/-/issues/583 +#[test] +fn subtree_hash3() { + check_subtree_hashes_are_equal( + r#"select ?, ? from "t" + option(sql_vdbe_max_steps = ?)"#, + vec![Value::Unsigned(1), Value::Unsigned(11), Value::Unsigned(10)], + r#"select ?, ? from "t""#, + vec![Value::Unsigned(1), Value::Unsigned(1)], + ); +} +*/ diff --git a/sbroad-core/src/frontend/sql.rs b/sbroad-core/src/frontend/sql.rs index b069a63557e1579a6c3d996281a999935c1dd68b..c8024d02a89c599b4b99dfd5ab23e17f97280b86 100644 --- a/sbroad-core/src/frontend/sql.rs +++ b/sbroad-core/src/frontend/sql.rs @@ -714,6 +714,9 @@ impl Ast for AbstractSyntaxTree { // Is it global for every `ValuesRow` met in the AST. let mut col_idx: usize = 0; + let mut met_tnt_param = false; + let mut met_pg_param = false; + let mut betweens: Vec<Between> = Vec::new(); // Ids of arithmetic expressions that have parentheses. let mut arith_expr_with_parentheses_ids: Vec<usize> = Vec::new(); @@ -1034,8 +1037,12 @@ impl Ast for AbstractSyntaxTree { .first() .expect("no children for sql_vdbe_max_steps option"); let ast_child_node = self.nodes.get_node(*ast_child_id)?; + let mut param_id = None; let val: Option<Value> = match ast_child_node.rule { - Type::Parameter => None, + Type::Parameter => { + param_id = Some(map.get(*ast_child_id)?); + None + } Type::Unsigned => { let v = { if let Some(str_value) = ast_child_node.value.as_ref() { @@ -1064,6 +1071,7 @@ impl Ast for AbstractSyntaxTree { plan.raw_options.push(OptionSpec { kind: OptionKind::VTableMaxRows, val, + param_id, }); } Type::SqlVdbeMaxSteps => { @@ -1072,8 +1080,12 @@ impl Ast for AbstractSyntaxTree { .first() .expect("no children for sql_vdbe_max_steps option"); let ast_child_node = self.nodes.get_node(*ast_child_id)?; + let mut param_id = None; let val: Option<Value> = match ast_child_node.rule { - Type::Parameter => None, + Type::Parameter => { + param_id = Some(map.get(*ast_child_id)?); + None + } Type::Unsigned => { let v = { if let Some(str_value) = ast_child_node.value.as_ref() { @@ -1102,11 +1114,62 @@ impl Ast for AbstractSyntaxTree { plan.raw_options.push(OptionSpec { kind: OptionKind::SqlVdbeMaxSteps, val, + param_id, }); } Type::Parameter => { + let ast_child_id = node.children.first().ok_or_else(|| { + SbroadError::Invalid( + Entity::AST, + Some(format!("parameter ast must have a child. id: {id}")), + ) + })?; + let plan_child_id = map.get(*ast_child_id)?; + map.add(id, plan_child_id); + } + Type::TntParameter => { + if met_pg_param { + return Err(SbroadError::UseOfBothParamsStyles); + } + met_tnt_param = true; map.add(id, plan.add_param()); } + Type::PgParameter => { + if met_tnt_param { + return Err(SbroadError::UseOfBothParamsStyles); + } + met_pg_param = true; + + let node_str = node + .value + .as_ref() + .expect("pg param ast node must have value"); + + // There is no child of pg parameter on purpose: + // if there is a child (Constant), we will add it to + // plan, and plan will now have extra nodes. This + // will cause the same expressions have different ids. + // And because of that we will get different subtree + // hashes in dispatch. + let value_idx = node_str[1..].parse::<usize>().map_err(|_| { + SbroadError::Invalid( + Entity::Query, + Some(format!("failed to parse parameter: {node_str}")), + ) + })?; + + if value_idx == 0 { + return Err(SbroadError::Invalid( + Entity::Query, + Some("$n parameters are indexed from 1!".into()), + )); + } + + let param_id = plan.add_param(); + map.add(id, param_id); + + plan.pg_params_map.insert(param_id, value_idx - 1); + } Type::Asterisk => { // We can get an asterisk only in projection. let ast_rel_list = self.get_referred_relational_nodes(id)?; diff --git a/sbroad-core/src/frontend/sql/ast.rs b/sbroad-core/src/frontend/sql/ast.rs index a37f986be25704eb2663e3deb415c0b6e9d88a28..0632c8207bd98a82d8a068cdbad55cedbc277a0a 100644 --- a/sbroad-core/src/frontend/sql/ast.rs +++ b/sbroad-core/src/frontend/sql/ast.rs @@ -115,6 +115,7 @@ pub enum Type { VTableMaxRows, Parameter, Parentheses, + PgParameter, Primary, PrimaryKey, PrimaryKeyColumn, @@ -161,6 +162,7 @@ pub enum Type { Table, TargetColumns, Timeout, + TntParameter, True, TypeAny, TypeBool, @@ -274,8 +276,9 @@ impl Type { Rule::Query => Ok(Type::Query), Rule::SqlVdbeMaxSteps => Ok(Type::SqlVdbeMaxSteps), Rule::VTableMaxRows => Ok(Type::VTableMaxRows), - Rule::Parameter => Ok(Type::Parameter), Rule::Parentheses => Ok(Type::Parentheses), + Rule::Parameter => Ok(Type::Parameter), + Rule::PgParameter => Ok(Type::PgParameter), Rule::Primary => Ok(Type::Primary), Rule::PrimaryKey => Ok(Type::PrimaryKey), Rule::PrimaryKeyColumn => Ok(Type::PrimaryKeyColumn), @@ -322,6 +325,7 @@ impl Type { Rule::Table => Ok(Type::Table), Rule::TargetColumns => Ok(Type::TargetColumns), Rule::Timeout => Ok(Type::Timeout), + Rule::TntParameter => Ok(Type::TntParameter), Rule::True => Ok(Type::True), Rule::TypeAny => Ok(Type::TypeAny), Rule::TypeBool => Ok(Type::TypeBool), diff --git a/sbroad-core/src/frontend/sql/ir/tests.rs b/sbroad-core/src/frontend/sql/ir/tests.rs index ab733f6cded6793b56aec5d1aa098b4df50ce726..a02d23f4b047472589b80dc31eb4a588b5bf2571 100644 --- a/sbroad-core/src/frontend/sql/ir/tests.rs +++ b/sbroad-core/src/frontend/sql/ir/tests.rs @@ -1179,6 +1179,100 @@ vtable_max_rows = 10 assert_eq!(expected_explain, plan.as_explain().unwrap()); } +#[test] +fn front_sql_pg_style_params1() { + let input = r#"select $1, $2, $1 from "t""#; + + let plan = sql_to_optimized_ir( + input, + vec![Value::Unsigned(1000), Value::String("hi".into())], + ); + let expected_explain = String::from( + r#"projection (1000::unsigned -> "COL_1", 'hi'::string -> "COL_2", 1000::unsigned -> "COL_3") + scan "t" +execution options: +sql_vdbe_max_steps = 45000 +vtable_max_rows = 5000 +"#, + ); + assert_eq!(expected_explain, plan.as_explain().unwrap()); +} + +#[test] +fn front_sql_pg_style_params2() { + let input = + r#"select $1, $2, $1 from "t" option(sql_vdbe_max_steps = $1, vtable_max_rows = $1)"#; + + let plan = sql_to_optimized_ir( + input, + vec![Value::Unsigned(1000), Value::String("hi".into())], + ); + let expected_explain = String::from( + r#"projection (1000::unsigned -> "COL_1", 'hi'::string -> "COL_2", 1000::unsigned -> "COL_3") + scan "t" +execution options: +sql_vdbe_max_steps = 1000 +vtable_max_rows = 1000 +"#, + ); + assert_eq!(expected_explain, plan.as_explain().unwrap()); +} + +#[test] +fn front_sql_pg_style_params3() { + let input = r#"select "a" + $1 from "t" + where "a" = $1 + group by "a" + $1 + having count("b") > $1 + option(sql_vdbe_max_steps = $1, vtable_max_rows = $1)"#; + + let plan = sql_to_optimized_ir(input, vec![Value::Unsigned(42)]); + let expected_explain = String::from( + r#"projection ("column_31"::unsigned -> "COL_1") + having ROW(sum(("count_45"::integer))::decimal) > ROW(42::unsigned) + group by ("column_31"::unsigned) output: ("column_31"::unsigned -> "column_31", "count_45"::integer -> "count_45") + motion [policy: segment([ref("column_31")])] + scan + projection (ROW("t"."a"::unsigned) + ROW(42::unsigned) -> "column_31", count(("t"."b"::unsigned))::integer -> "count_45") + group by (ROW("t"."a"::unsigned) + ROW(42::unsigned)) output: ("t"."a"::unsigned -> "a", "t"."b"::unsigned -> "b", "t"."c"::unsigned -> "c", "t"."d"::unsigned -> "d", "t"."bucket_id"::unsigned -> "bucket_id") + selection ROW("t"."a"::unsigned) = ROW(42::unsigned) + scan "t" +execution options: +sql_vdbe_max_steps = 42 +vtable_max_rows = 42 +"#, + ); + assert_eq!(expected_explain, plan.as_explain().unwrap()); +} + +#[test] +fn front_sql_pg_style_params4() { + let input = r#"select $1, ? from "t""#; + + let metadata = &RouterConfigurationMock::new(); + let ast = AbstractSyntaxTree::new(input).unwrap(); + let err = ast.resolve_metadata(metadata).unwrap_err(); + + assert_eq!( + "invalid parameters usage. Got $n and ? parameters in one query!", + err.to_string() + ); +} + +#[test] +fn front_sql_pg_style_params5() { + let input = r#"select $0 from "t""#; + + let metadata = &RouterConfigurationMock::new(); + let ast = AbstractSyntaxTree::new(input).unwrap(); + let err = ast.resolve_metadata(metadata).unwrap_err(); + + assert_eq!( + "invalid query: $n parameters are indexed from 1!", + err.to_string() + ); +} + #[test] fn front_sql_option_defaults() { let input = r#"select * from "t" where "a" = ? and "b" = ?"#; diff --git a/sbroad-core/src/frontend/sql/query.pest b/sbroad-core/src/frontend/sql/query.pest index b5defce48569bbccc2d117efe06703b62cfba3a4..955cfbff726c1330fbbade10870bfb9513aad5e0 100644 --- a/sbroad-core/src/frontend/sql/query.pest +++ b/sbroad-core/src/frontend/sql/query.pest @@ -253,7 +253,9 @@ Value = _{ Parameter | True | False | Null | Double | Decimal | Unsigned | Integ | (^"row" ~ "(" ~ RowElem ~ ("," ~ RowElem)* ~ ")") } RowElem = _{ Concat | Cast | Row | Function | Value } - Parameter = @{ "?" } + Parameter = { PgParameter | TntParameter } + TntParameter = @{ "?" } + PgParameter = @{ "$" ~ Unsigned } EOF = { EOI | ";" } WHITESPACE = _{ " " | "\t" | "\n" | "\r\n" } diff --git a/sbroad-core/src/ir.rs b/sbroad-core/src/ir.rs index 89b3dd812a4a54d715d072f0a3c5efc2a0fad695..7bfd6d17af1776ec05fd52ad0d204ff05c4dc045 100644 --- a/sbroad-core/src/ir.rs +++ b/sbroad-core/src/ir.rs @@ -242,6 +242,7 @@ impl Slices { pub struct OptionSpec { pub kind: OptionKind, pub val: Option<Value>, + pub param_id: Option<usize>, } #[derive(Clone, PartialEq, Eq, Debug, Hash, Deserialize, Serialize)] @@ -355,6 +356,9 @@ impl Options { } } +pub type NodeId = usize; +pub type ValueIdx = usize; + /// Logical plan tree structure. #[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] pub struct Plan { @@ -387,6 +391,11 @@ pub struct Plan { /// for storing the order of options in `Option` clause, after `bind_params` is /// called this field is not used and becomes empty. pub raw_options: Vec<OptionSpec>, + /// Mapping between parameter plan id and corresponding value position in + /// in values list. This is needed only for handling PG-like parameters + /// in `bind_params` after which it becomes `None`. + /// If query uses tnt-like params, then this field is always `None`. + pub pg_params_map: HashMap<NodeId, ValueIdx>, /// SQL options. Initiliazed to defaults upon IR creation. Then bound to actual /// values after `bind_params` these options are set to their actual values. /// See `apply_options`. @@ -445,6 +454,7 @@ impl Plan { raw_options: vec![], options: Options::default(), version_map: TableVersionMap::new(), + pg_params_map: HashMap::new(), } } diff --git a/sbroad-core/src/ir/api/parameter.rs b/sbroad-core/src/ir/api/parameter.rs index 4d56bec13ac0631c8a9028649068f5856f58cf3a..c46268c420febf550280b322493dded208bac41b 100644 --- a/sbroad-core/src/ir/api/parameter.rs +++ b/sbroad-core/src/ir/api/parameter.rs @@ -43,9 +43,9 @@ impl Plan { /// - Internal errors. #[allow(clippy::too_many_lines)] #[otm_child_span("plan.bind")] - pub fn bind_params(&mut self, mut params: Vec<Value>) -> Result<(), SbroadError> { + pub fn bind_params(&mut self, mut values: Vec<Value>) -> Result<(), SbroadError> { // Nothing to do here. - if params.is_empty() { + if values.is_empty() { return Ok(()); } @@ -57,13 +57,69 @@ impl Plan { let mut binded_params_counter = 0; if !self.raw_options.is_empty() { - binded_params_counter = self.bind_option_params(&mut params)?; + binded_params_counter = self.bind_option_params(&mut values)?; + } + + // Gather all parameter nodes from the tree to a hash set. + // `param_node_ids` is used during first plan traversal (`row_ids` populating). + // `param_set_params` is used during second plan traversal (nodes transformation). + let mut param_node_ids = self.get_param_set(); + let mut param_node_ids_cloned = param_node_ids.clone(); + + let tnt_params_style = self.pg_params_map.is_empty(); + + let mut pg_params_map = std::mem::take(&mut self.pg_params_map); + + if !tnt_params_style { + // Due to how we calculate hash for plan subtree and the + // fact that pg parameters can refer to same value multiple + // times we currently copy params that are referred more + // than once in order to get the same hash. + // See https://git.picodata.io/picodata/picodata/sbroad/-/issues/583 + let mut used_values = vec![false; values.len()]; + let invalid_idx = |param_id: usize, value_idx: usize| { + SbroadError::Invalid( + Entity::Plan, + Some(format!( + "out of bounds value index {value_idx} for pg parameter {param_id}" + )), + ) + }; + + // NB: we can't use `param_node_ids`, we need to traverse + // parameters in the same order they will be bound, + // otherwise we may get different hashes for plans + // with tnt and pg parameters. See `subtree_hash*` tests, + for (_, param_id) in &nodes { + if !matches!(self.get_node(*param_id)?, Node::Parameter) { + continue; + } + let value_idx = *pg_params_map.get(param_id).ok_or(SbroadError::Invalid( + Entity::Plan, + Some(format!( + "value index not found for parameter with id: {param_id}", + )), + ))?; + if used_values.get(value_idx).copied().unwrap_or(true) { + let Some(value) = values.get(value_idx) else { + return Err(invalid_idx(*param_id, value_idx)) + }; + values.push(value.clone()); + pg_params_map + .entry(*param_id) + .and_modify(|value_idx| *value_idx = values.len() - 1); + } else if let Some(used) = used_values.get_mut(value_idx) { + *used = true; + } else { + return Err(invalid_idx(*param_id, value_idx)); + } + } } // Transform parameters to values (plan constants). The result values are stored in the // opposite to parameters order. - let mut value_ids: Vec<usize> = Vec::with_capacity(params.len()); - while let Some(param) = params.pop() { + let mut value_ids: Vec<usize> = Vec::with_capacity(values.len()); + while let Some(param) = values.pop() { value_ids.push(self.add_const(param)); } @@ -73,13 +129,7 @@ impl Plan { let mut row_ids: HashMap<usize, usize, RandomState> = HashMap::with_hasher(RandomState::new()); - // Gather all parameter nodes from the tree to a hash set. - // `param_node_ids` is used during first plan traversal (`row_ids` populating). - // `param_set_params` is used during second plan traversal (nodes transformation). - let mut param_node_ids = self.get_param_set(); - let mut param_node_ids_cloned = param_node_ids.clone(); - - if param_node_ids.len() - binded_params_counter > value_ids.len() { + if tnt_params_style && param_node_ids.len() - binded_params_counter > value_ids.len() { return Err(SbroadError::Invalid( Entity::Value, Some(format!( @@ -90,41 +140,54 @@ impl Plan { )); } - // Closure to retrieve a corresponding value for a parameter node. - let get_value = |pos: usize| -> Result<usize, SbroadError> { - let val_id = value_ids.get(pos).ok_or_else(|| { - SbroadError::NotFound(Entity::Node, format!("(Parameter) in position {pos}")) - })?; - Ok(*val_id) - }; - - // After binding parameters we need to recalculate expression types. - let mut new_types = HashMap::with_capacity_and_hasher(nodes.len(), RandomState::new()); - // Populate rows. // Number of parameters - `idx` - 1 = index in params we are currently binding. // Initially pointing to nowhere. let mut idx = value_ids.len(); + + let get_value = |param_id: usize, idx: usize| -> Result<usize, SbroadError> { + let value_idx = if tnt_params_style { + // in case non-pg params are used, + // idx is the correct position + idx + } else { + value_ids.len() + - 1 + - *pg_params_map.get(¶m_id).ok_or_else(|| { + SbroadError::Invalid( + Entity::Plan, + Some(format!( + "value index not found for parameter with id: {param_id}", + )), + ) + })? + }; + let val_id = value_ids.get(value_idx).ok_or_else(|| { + SbroadError::NotFound(Entity::Node, format!("(Parameter) in position {value_idx}")) + })?; + Ok(*val_id) + }; + for (_, id) in &nodes { let node = self.get_node(*id)?; match node { Node::Relational(rel) => match rel { - Relational::Selection { + Relational::Having { filter: ref param_id, .. } - | Relational::Join { - condition: ref param_id, + | Relational::Selection { + filter: ref param_id, .. } - | Relational::Projection { - output: ref param_id, + | Relational::Join { + condition: ref param_id, .. } => { if param_node_ids.take(param_id).is_some() { - idx -= 1; - let val_id = get_value(idx)?; - row_ids.insert(idx, self.nodes.add_row(vec![val_id], None)); + idx = idx.saturating_sub(1); + let val_id = get_value(*param_id, idx)?; + row_ids.insert(*param_id, self.nodes.add_row(vec![val_id], None)); } } _ => {} @@ -143,7 +206,7 @@ impl Plan { .. } => { if param_node_ids.take(param_id).is_some() { - idx -= 1; + idx = idx.saturating_sub(1); } } Expression::Bool { @@ -162,9 +225,9 @@ impl Plan { } => { for param_id in &[*left, *right] { if param_node_ids.take(param_id).is_some() { - idx -= 1; - let val_id = get_value(idx)?; - row_ids.insert(idx, self.nodes.add_row(vec![val_id], None)); + idx = idx.saturating_sub(1); + let val_id = get_value(*param_id, idx)?; + row_ids.insert(*param_id, self.nodes.add_row(vec![val_id], None)); } } } @@ -176,24 +239,22 @@ impl Plan { if param_node_ids.take(param_id).is_some() { // Parameter is already under row/function so that we don't // have to cover it with `add_row` call. - idx -= 1; + idx = idx.saturating_sub(1); } } } - Expression::Reference { .. } => { - // Remember to recalculate type. - new_types.insert(id, expr.recalculate_type(self)?); - } - Expression::Constant { .. } | Expression::CountAsterisk => {} + Expression::Reference { .. } + | Expression::Constant { .. } + | Expression::CountAsterisk => {} }, Node::Parameter | Node::Ddl(..) | Node::Acl(..) => {} } } // Closure to retrieve a corresponding row for a parameter node. - let get_row = |idx: usize| -> Result<usize, SbroadError> { - let row_id = row_ids.get(&idx).ok_or_else(|| { - SbroadError::NotFound(Entity::Node, format!("(Row) at position {idx}")) + let get_row = |param_id: usize| -> Result<usize, SbroadError> { + let row_id = row_ids.get(¶m_id).ok_or_else(|| { + SbroadError::NotFound(Entity::Node, format!("(Row) at position {param_id}")) })?; Ok(*row_id) }; @@ -201,24 +262,41 @@ impl Plan { // Replace parameters in the plan. idx = value_ids.len(); for (_, id) in &nodes { + // Before binding, references that referred to + // parameters had scalar type (by default), + // but in fact they may refer to different stuff. + { + let mut new_type = None; + if let Node::Expression(expr) = self.get_node(*id)? { + if let Expression::Reference { .. } = expr { + new_type = Some(expr.recalculate_type(self)?); + } + } + if let Some(new_type) = new_type { + let expr = self.get_mut_expression_node(*id)?; + expr.set_ref_type(new_type); + continue; + } + } + let node = self.get_mut_node(*id)?; match node { Node::Relational(rel) => match rel { - Relational::Selection { + Relational::Having { filter: ref mut param_id, .. } - | Relational::Join { - condition: ref mut param_id, + | Relational::Selection { + filter: ref mut param_id, .. } - | Relational::Projection { - output: ref mut param_id, + | Relational::Join { + condition: ref mut param_id, .. } => { if param_node_ids_cloned.take(param_id).is_some() { - idx -= 1; - let row_id = get_row(idx)?; + idx = idx.saturating_sub(1); + let row_id = get_row(*param_id)?; *param_id = row_id; } } @@ -238,8 +316,8 @@ impl Plan { .. } => { if param_node_ids_cloned.take(param_id).is_some() { - idx -= 1; - let val_id = get_value(idx)?; + idx = idx.saturating_sub(1); + let val_id = get_value(*param_id, idx)?; *param_id = val_id; } } @@ -259,8 +337,8 @@ impl Plan { } => { for param_id in &mut [left, right].iter_mut() { if param_node_ids_cloned.take(param_id).is_some() { - idx -= 1; - let row_id = get_row(idx)?; + idx = idx.saturating_sub(1); + let row_id = get_row(**param_id)?; **param_id = row_id; } } @@ -272,26 +350,15 @@ impl Plan { } => { for param_id in list { if param_node_ids_cloned.take(param_id).is_some() { - idx -= 1; - let val_id = get_value(idx)?; + idx = idx.saturating_sub(1); + let val_id = get_value(*param_id, idx)?; *param_id = val_id; } } } - Expression::Reference { .. } => { - // Update type. - let new_type = new_types - .get(id) - .ok_or_else(|| { - SbroadError::NotFound( - Entity::Node, - format!("failed to get type for node {id}"), - ) - })? - .clone(); - expr.set_ref_type(new_type); - } - Expression::Constant { .. } | Expression::CountAsterisk => {} + Expression::Reference { .. } + | Expression::Constant { .. } + | Expression::CountAsterisk => {} }, Node::Parameter | Node::Ddl(..) | Node::Acl(..) => {} } @@ -319,7 +386,30 @@ impl Plan { let mut binded_params_counter = 0usize; for opt in self.raw_options.iter_mut().rev() { if opt.val.is_none() { - if let Some(v) = params.pop() { + if !self.pg_params_map.is_empty() { + // PG-like params syntax + let Some(param_id) = opt.param_id else { + return Err(SbroadError::Invalid( + Entity::Plan, + Some(format!("no param id for option: {opt:?}")) + )) + }; + let value_idx = *self.pg_params_map.get(¶m_id).ok_or_else(|| { + SbroadError::Invalid( + Entity::Plan, + Some(format!("no value idx in map for option parameter: {opt:?}")), + ) + })?; + let value = params.get(value_idx).ok_or_else(|| { + SbroadError::Invalid( + Entity::Plan, + Some(format!( + "invalid value idx {value_idx}, for option: {opt:?}" + )), + ) + })?; + opt.val = Some(value.clone()); + } else if let Some(v) = params.pop() { binded_params_counter += 1; opt.val = Some(v); } else { diff --git a/sbroad-core/tests/artifactory/backend/sql/tree/arbitrary_projection_plan.yaml b/sbroad-core/tests/artifactory/backend/sql/tree/arbitrary_projection_plan.yaml index 2cd3f545e17415bb04e2efdeea80891d47593fbf..2215c62c04054e3154322e2a4de97b9583df64f2 100644 --- a/sbroad-core/tests/artifactory/backend/sql/tree/arbitrary_projection_plan.yaml +++ b/sbroad-core/tests/artifactory/backend/sql/tree/arbitrary_projection_plan.yaml @@ -216,6 +216,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/backend/sql/tree/arithmetic_projection_plan.yaml b/sbroad-core/tests/artifactory/backend/sql/tree/arithmetic_projection_plan.yaml index 83eb9eee4336ea7d3a9517ea35f25830cc6eeeeb..9c2df340e5209b9ce8819af51708a3801ce58a20 100644 --- a/sbroad-core/tests/artifactory/backend/sql/tree/arithmetic_projection_plan.yaml +++ b/sbroad-core/tests/artifactory/backend/sql/tree/arithmetic_projection_plan.yaml @@ -289,6 +289,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/backend/sql/tree/arithmetic_selection_plan.yaml b/sbroad-core/tests/artifactory/backend/sql/tree/arithmetic_selection_plan.yaml index 2c05c97084470741980741354e001f3c4263952e..2138945487741c19f545a5a323bb2bc6aa29d8c7 100644 --- a/sbroad-core/tests/artifactory/backend/sql/tree/arithmetic_selection_plan.yaml +++ b/sbroad-core/tests/artifactory/backend/sql/tree/arithmetic_selection_plan.yaml @@ -416,6 +416,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/backend/sql/tree/sql_order_selection.yaml b/sbroad-core/tests/artifactory/backend/sql/tree/sql_order_selection.yaml index 04e6a7d25c25c2f4c395bd7954f46a85f877e9a3..175952afba9cd2069b19f8f56a8226a06e4b47d1 100644 --- a/sbroad-core/tests/artifactory/backend/sql/tree/sql_order_selection.yaml +++ b/sbroad-core/tests/artifactory/backend/sql/tree/sql_order_selection.yaml @@ -131,6 +131,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/distribution/join_unite_keys.yaml b/sbroad-core/tests/artifactory/ir/distribution/join_unite_keys.yaml index dfd291714c92e94a98ceffb3de71d1241ae163f5..95030453fd987de156b481a6ecc0ac6a960c2fac 100644 --- a/sbroad-core/tests/artifactory/ir/distribution/join_unite_keys.yaml +++ b/sbroad-core/tests/artifactory/ir/distribution/join_unite_keys.yaml @@ -200,6 +200,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/distribution/shrink_dist_key_1.yaml b/sbroad-core/tests/artifactory/ir/distribution/shrink_dist_key_1.yaml index 3ab0b75a2ca74724ac238a7da75ea8e419268134..1dd3800cb4c46c45271641fa279065393eadd5a9 100644 --- a/sbroad-core/tests/artifactory/ir/distribution/shrink_dist_key_1.yaml +++ b/sbroad-core/tests/artifactory/ir/distribution/shrink_dist_key_1.yaml @@ -118,6 +118,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/distribution/shrink_dist_key_2.yaml b/sbroad-core/tests/artifactory/ir/distribution/shrink_dist_key_2.yaml index ac1e8928f41933cece44ad608da96da284a5efc0..f41e0cf0f62d31a57df0b2693772a5226a3090b1 100644 --- a/sbroad-core/tests/artifactory/ir/distribution/shrink_dist_key_2.yaml +++ b/sbroad-core/tests/artifactory/ir/distribution/shrink_dist_key_2.yaml @@ -106,6 +106,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/distribution/shuffle_dist_key.yaml b/sbroad-core/tests/artifactory/ir/distribution/shuffle_dist_key.yaml index e42bf59c5f69543957a4afff0a36a91bf2448e02..f05915f5f964e53b79cbaa2c41f8f636566df9ac 100644 --- a/sbroad-core/tests/artifactory/ir/distribution/shuffle_dist_key.yaml +++ b/sbroad-core/tests/artifactory/ir/distribution/shuffle_dist_key.yaml @@ -118,6 +118,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/distribution/union_fallback_to_random.yaml b/sbroad-core/tests/artifactory/ir/distribution/union_fallback_to_random.yaml index d64da381e05c421ebebf7c9b618bbf2990e75bf6..fd9a0b065924c64d584436be62d8d9f9709180c6 100644 --- a/sbroad-core/tests/artifactory/ir/distribution/union_fallback_to_random.yaml +++ b/sbroad-core/tests/artifactory/ir/distribution/union_fallback_to_random.yaml @@ -139,6 +139,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/distribution/union_preserve_dist.yaml b/sbroad-core/tests/artifactory/ir/distribution/union_preserve_dist.yaml index b4a7d0a76cf61c1aafdd05c43c3151166621b67f..d53a68a4ec1c6f1e553833a83ef9350b87c876f9 100644 --- a/sbroad-core/tests/artifactory/ir/distribution/union_preserve_dist.yaml +++ b/sbroad-core/tests/artifactory/ir/distribution/union_preserve_dist.yaml @@ -139,6 +139,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/operator/join.yaml b/sbroad-core/tests/artifactory/ir/operator/join.yaml index bad216b002b311433b174133e5c551f1353e8251..f18ccd711369be206e65a4f32401f85be352fcd6 100644 --- a/sbroad-core/tests/artifactory/ir/operator/join.yaml +++ b/sbroad-core/tests/artifactory/ir/operator/join.yaml @@ -200,6 +200,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/operator/projection.yaml b/sbroad-core/tests/artifactory/ir/operator/projection.yaml index 0423db664d523b2816abd739c76b491751acdf25..500d6c9a64683bb06c1f11d2a2703c3db093b98d 100644 --- a/sbroad-core/tests/artifactory/ir/operator/projection.yaml +++ b/sbroad-core/tests/artifactory/ir/operator/projection.yaml @@ -90,6 +90,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/operator/scan_rel.yaml b/sbroad-core/tests/artifactory/ir/operator/scan_rel.yaml index 303b5686d36f86a43005e4bee1cd86452527d400..5f0e54fdc81e0dc44d203ff8b3641c3d5e7d3184 100644 --- a/sbroad-core/tests/artifactory/ir/operator/scan_rel.yaml +++ b/sbroad-core/tests/artifactory/ir/operator/scan_rel.yaml @@ -90,6 +90,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/operator/selection.yaml b/sbroad-core/tests/artifactory/ir/operator/selection.yaml index 23241ad4187a8bc22016a74dc792266a8f6c2b18..621e1eeaedfbf3d04d0e5103a371242499e586a1 100644 --- a/sbroad-core/tests/artifactory/ir/operator/selection.yaml +++ b/sbroad-core/tests/artifactory/ir/operator/selection.yaml @@ -172,6 +172,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/operator/selection_with_sub_query.yaml b/sbroad-core/tests/artifactory/ir/operator/selection_with_sub_query.yaml index 832d8977c8c514c20314e643f1df6e3aaeb1ceed..5b77212a7d33ddbbe150438faeadc0993216b8dc 100644 --- a/sbroad-core/tests/artifactory/ir/operator/selection_with_sub_query.yaml +++ b/sbroad-core/tests/artifactory/ir/operator/selection_with_sub_query.yaml @@ -176,6 +176,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/operator/sub_query.yaml b/sbroad-core/tests/artifactory/ir/operator/sub_query.yaml index 2193f621988692470909672829ceb5af97078ca5..8f3ced1544ae8722c48a21a0bd4a99d9769a1294 100644 --- a/sbroad-core/tests/artifactory/ir/operator/sub_query.yaml +++ b/sbroad-core/tests/artifactory/ir/operator/sub_query.yaml @@ -101,6 +101,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/plan_no_top.yaml b/sbroad-core/tests/artifactory/ir/plan_no_top.yaml index 59e0b3318e75c9b104150a95b10059d0abd1cdcb..666a51b2203789412628137426ac8d00117e756a 100644 --- a/sbroad-core/tests/artifactory/ir/plan_no_top.yaml +++ b/sbroad-core/tests/artifactory/ir/plan_no_top.yaml @@ -48,6 +48,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/plan_oor_top.yaml b/sbroad-core/tests/artifactory/ir/plan_oor_top.yaml index 036abe88d29c4b9289c91dd219c447cc63be5712..b1366fb41f5409253a8e8ae01413cfad695a0bc3 100644 --- a/sbroad-core/tests/artifactory/ir/plan_oor_top.yaml +++ b/sbroad-core/tests/artifactory/ir/plan_oor_top.yaml @@ -48,6 +48,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/transformation/redistribution/full_motion_less_for_sub_query.yaml b/sbroad-core/tests/artifactory/ir/transformation/redistribution/full_motion_less_for_sub_query.yaml index b885c537809c4384a7c271fa243fe75ad72daf01..648176a5dc02e31a91a7c488f7a56d25e177d4b2 100644 --- a/sbroad-core/tests/artifactory/ir/transformation/redistribution/full_motion_less_for_sub_query.yaml +++ b/sbroad-core/tests/artifactory/ir/transformation/redistribution/full_motion_less_for_sub_query.yaml @@ -228,6 +228,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/transformation/redistribution/full_motion_non_segment_outer_for_sub_query.yaml b/sbroad-core/tests/artifactory/ir/transformation/redistribution/full_motion_non_segment_outer_for_sub_query.yaml index 13d0ea7aa6ede6b4b4407ec8bb0d943d2030c4d3..1a64ae74ee472225da1c7e4dd87460e46eeadbf2 100644 --- a/sbroad-core/tests/artifactory/ir/transformation/redistribution/full_motion_non_segment_outer_for_sub_query.yaml +++ b/sbroad-core/tests/artifactory/ir/transformation/redistribution/full_motion_non_segment_outer_for_sub_query.yaml @@ -248,6 +248,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/transformation/redistribution/local_sub_query.yaml b/sbroad-core/tests/artifactory/ir/transformation/redistribution/local_sub_query.yaml index 01167fd074766725de015d4aa16286527b83edf7..d0f08e05540218cd9be897a1462e7f0e835b3c24 100644 --- a/sbroad-core/tests/artifactory/ir/transformation/redistribution/local_sub_query.yaml +++ b/sbroad-core/tests/artifactory/ir/transformation/redistribution/local_sub_query.yaml @@ -201,6 +201,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/transformation/redistribution/multiple_sub_queries.yaml b/sbroad-core/tests/artifactory/ir/transformation/redistribution/multiple_sub_queries.yaml index 6d987410864e8ae95a91736905e89a1e425df52d..cf2ddebdf05b01e75904e2e7ac0eea836a3035f3 100644 --- a/sbroad-core/tests/artifactory/ir/transformation/redistribution/multiple_sub_queries.yaml +++ b/sbroad-core/tests/artifactory/ir/transformation/redistribution/multiple_sub_queries.yaml @@ -390,6 +390,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: diff --git a/sbroad-core/tests/artifactory/ir/transformation/redistribution/segment_motion_for_sub_query.yaml b/sbroad-core/tests/artifactory/ir/transformation/redistribution/segment_motion_for_sub_query.yaml index 49d47415cbbcf5ed9a6e3f2daa76b1a21e3f0af9..19ad8c20d196f9156dd82b75c6de6066f2ee7bd3 100644 --- a/sbroad-core/tests/artifactory/ir/transformation/redistribution/segment_motion_for_sub_query.yaml +++ b/sbroad-core/tests/artifactory/ir/transformation/redistribution/segment_motion_for_sub_query.yaml @@ -239,6 +239,7 @@ undo: log: {} constants: {} raw_options: [] +pg_params_map: {} options: vtable_max_rows: 5000 execute_options: