From 2ba636c249598348a15e890370c194ecc098586a Mon Sep 17 00:00:00 2001 From: Denis Smirnov <sd@picodata.io> Date: Fri, 29 Dec 2023 11:15:18 +0700 Subject: [PATCH] fix: insert column names --- sbroad-core/src/frontend/sql.rs | 10 +-- sbroad-core/src/frontend/sql/ir/tests.rs | 4 ++ .../src/frontend/sql/ir/tests/insert.rs | 61 +++++++++++++++++++ sbroad-core/src/ir/api/parameter.rs | 1 - sbroad-core/src/ir/operator.rs | 4 +- sbroad-core/src/ir/operator/tests.rs | 15 +++-- 6 files changed, 82 insertions(+), 13 deletions(-) create mode 100644 sbroad-core/src/frontend/sql/ir/tests/insert.rs diff --git a/sbroad-core/src/frontend/sql.rs b/sbroad-core/src/frontend/sql.rs index 5a0e5ef941..463c67a79b 100644 --- a/sbroad-core/src/frontend/sql.rs +++ b/sbroad-core/src/frontend/sql.rs @@ -1843,12 +1843,14 @@ impl Ast for AbstractSyntaxTree { let ast_child = self.nodes.get_node(*ast_child_id)?; let plan_insert_id = if let Type::TargetColumns = ast_child.rule { // insert into t (a, b, c) ... - let mut selected_col_names: Vec<&str> = + let mut selected_col_names: Vec<String> = Vec::with_capacity(ast_child.children.len()); for col_id in &ast_child.children { let col = self.nodes.get_node(*col_id)?; if let Type::ColumnName = col.rule { - selected_col_names.push(parse_string_value_node(self, *col_id)?); + selected_col_names.push(normalize_name_from_sql( + parse_string_value_node(self, *col_id)?, + )); } else { return Err(SbroadError::Invalid( Entity::Type, @@ -1867,9 +1869,7 @@ impl Ast for AbstractSyntaxTree { if let ColumnRole::Sharding = column.get_role() { continue; } - if !column.is_nullable - && !selected_col_names.contains(&column.name.as_str()) - { + if !column.is_nullable && !selected_col_names.contains(&column.name) { return Err(SbroadError::Invalid( Entity::Column, Some(format!( diff --git a/sbroad-core/src/frontend/sql/ir/tests.rs b/sbroad-core/src/frontend/sql/ir/tests.rs index f025ea88a2..ab733f6cde 100644 --- a/sbroad-core/src/frontend/sql/ir/tests.rs +++ b/sbroad-core/src/frontend/sql/ir/tests.rs @@ -2812,9 +2812,13 @@ fn assert_explain_eq(query: &str, params: Vec<Value>, expected: &str) { assert_eq!(expected, actual); } +#[cfg(test)] mod global; #[cfg(test)] +mod insert; +#[cfg(test)] mod params; +#[cfg(test)] mod single; #[cfg(test)] mod update; diff --git a/sbroad-core/src/frontend/sql/ir/tests/insert.rs b/sbroad-core/src/frontend/sql/ir/tests/insert.rs new file mode 100644 index 0000000000..43c1707ff7 --- /dev/null +++ b/sbroad-core/src/frontend/sql/ir/tests/insert.rs @@ -0,0 +1,61 @@ +use crate::ir::transformation::helpers::sql_to_optimized_ir; +use crate::ir::value::Value; +use pretty_assertions::assert_eq; + +#[test] +fn insert1() { + let pattern = r#"INSERT INTO "test_space"("id", first_name) VALUES(?, ?)"#; + let plan = sql_to_optimized_ir(pattern, vec![Value::from(1_i64), Value::from("test")]); + + let expected_explain = String::from( + r#"insert "test_space" on conflict: fail + motion [policy: segment([ref("COLUMN_1")])] + values + value row (data=ROW(1::integer, 'test'::string)) +execution options: +sql_vdbe_max_steps = 45000 +vtable_max_rows = 5000 +"#, + ); + + assert_eq!(expected_explain, plan.as_explain().unwrap()); +} + +#[test] +fn insert2() { + let pattern = r#"INSERT INTO "test_space"("id", "FIRST_NAME") VALUES(1, 'test')"#; + let plan = sql_to_optimized_ir(pattern, vec![]); + + let expected_explain = String::from( + r#"insert "test_space" on conflict: fail + motion [policy: segment([ref("COLUMN_1")])] + values + value row (data=ROW(1::unsigned, 'test'::string)) +execution options: +sql_vdbe_max_steps = 45000 +vtable_max_rows = 5000 +"#, + ); + + assert_eq!(expected_explain, plan.as_explain().unwrap()); +} + +#[test] +fn insert3() { + let pattern = r#"INSERT INTO "test_space"("id", "sys_op") + SELECT "id", "id" FROM "test_space""#; + let plan = sql_to_optimized_ir(pattern, vec![]); + + let expected_explain = String::from( + r#"insert "test_space" on conflict: fail + motion [policy: segment([ref("id")])] + projection ("test_space"."id"::unsigned -> "id", "test_space"."id"::unsigned -> "id") + scan "test_space" +execution options: +sql_vdbe_max_steps = 45000 +vtable_max_rows = 5000 +"#, + ); + + assert_eq!(expected_explain, plan.as_explain().unwrap()); +} diff --git a/sbroad-core/src/ir/api/parameter.rs b/sbroad-core/src/ir/api/parameter.rs index 7dbe18b7e3..4d56bec13a 100644 --- a/sbroad-core/src/ir/api/parameter.rs +++ b/sbroad-core/src/ir/api/parameter.rs @@ -181,7 +181,6 @@ impl Plan { } } Expression::Reference { .. } => { - println!("expr: {expr:?}, id: {id}"); // Remember to recalculate type. new_types.insert(id, expr.recalculate_type(self)?); } diff --git a/sbroad-core/src/ir/operator.rs b/sbroad-core/src/ir/operator.rs index 98056533bb..80f11632c2 100644 --- a/sbroad-core/src/ir/operator.rs +++ b/sbroad-core/src/ir/operator.rs @@ -1105,7 +1105,7 @@ impl Plan { &mut self, relation: &str, child: usize, - columns: &[&str], + columns: &[String], conflict_strategy: ConflictStrategy, ) -> Result<usize, SbroadError> { let rel = self.relations.get(relation).ok_or_else(|| { @@ -1126,7 +1126,7 @@ impl Plan { }); let mut cols: Vec<usize> = Vec::with_capacity(names.len()); for name in columns { - match names.get(name) { + match names.get(name.as_str()) { Some((&ColumnRole::User, pos)) => cols.push(*pos), Some((&ColumnRole::Sharding, _)) => { return Err(SbroadError::FailedTo( diff --git a/sbroad-core/src/ir/operator/tests.rs b/sbroad-core/src/ir/operator/tests.rs index 72026e8d5b..43c3a4358e 100644 --- a/sbroad-core/src/ir/operator/tests.rs +++ b/sbroad-core/src/ir/operator/tests.rs @@ -299,7 +299,7 @@ fn insert() { assert_eq!( SbroadError::NotFound(Entity::Table, "t4 among plan relations".into()), - plan.add_insert("t4", scan_t1_id, &["a"], ConflictStrategy::default()) + plan.add_insert("t4", scan_t1_id, &["a".into()], ConflictStrategy::default()) .unwrap_err() ); @@ -312,7 +312,7 @@ fn insert() { plan.add_insert( "t2", scan_t1_id, - &["a", "b", "c"], + &["a".into(), "b".into(), "c".into()], ConflictStrategy::default() ) .unwrap_err() @@ -322,11 +322,16 @@ fn insert() { SbroadError::UnexpectedNumberOfValues( "invalid number of values: 1. Table t2 expects 2 column(s).".into() ), - plan.add_insert("t2", scan_t1_id, &["a", "b"], ConflictStrategy::default()) - .unwrap_err() + plan.add_insert( + "t2", + scan_t1_id, + &["a".into(), "b".into()], + ConflictStrategy::default() + ) + .unwrap_err() ); - plan.add_insert("t1", scan_t1_id, &["a"], ConflictStrategy::default()) + plan.add_insert("t1", scan_t1_id, &["a".into()], ConflictStrategy::default()) .unwrap(); } -- GitLab