From d0e54e0d646e04b94f1dca4da879ae57bce3b9c9 Mon Sep 17 00:00:00 2001
From: Denis Smirnov <sd@picodata.io>
Date: Wed, 12 Oct 2022 12:59:32 +0700
Subject: [PATCH] fix: incorrect column name generation for empty results
---
sbroad-core/src/backend/sql/ir.rs | 16 +--
sbroad-core/src/executor/engine/mock.rs | 13 +++
sbroad-core/src/executor/tests.rs | 3 +
.../src/executor/tests/empty_motion.rs | 97 +++++++++++++++++++
4 files changed, 121 insertions(+), 8 deletions(-)
create mode 100644 sbroad-core/src/executor/tests/empty_motion.rs
diff --git a/sbroad-core/src/backend/sql/ir.rs b/sbroad-core/src/backend/sql/ir.rs
index 7697bdf7a9..7046963270 100644
--- a/sbroad-core/src/backend/sql/ir.rs
+++ b/sbroad-core/src/backend/sql/ir.rs
@@ -250,12 +250,14 @@ impl ExecutionPlan {
SyntaxData::VTable(vtable) => {
let cols_count = vtable.get_columns().len();
- let cols = |base_idx| {
+ let cols = |base_idx: &mut usize| {
vtable
.get_columns()
.iter()
- .enumerate()
- .map(|(i, c)| format!("COLUMN_{} as \"{}\"", base_idx + i, c.name))
+ .map(|c| {
+ *base_idx += 1;
+ format!("COLUMN_{} as \"{}\"", base_idx, c.name)
+ })
.collect::<Vec<String>>()
.join(",")
};
@@ -278,8 +280,6 @@ impl ExecutionPlan {
};
if tuples.is_empty() {
- anonymous_col_idx_base += 1;
-
let values = (0..cols_count)
.map(|_| "null")
.collect::<Vec<&str>>()
@@ -288,7 +288,7 @@ impl ExecutionPlan {
write!(
sql,
"SELECT {} FROM (VALUES ({})) WHERE FALSE",
- cols(anonymous_col_idx_base),
+ cols(&mut anonymous_col_idx_base),
values
)
.map_err(|e| QueryPlannerError::CustomError(e.to_string()))?;
@@ -299,12 +299,12 @@ impl ExecutionPlan {
.collect::<Vec<String>>()
.join(",");
- anonymous_col_idx_base += cols_count * tuples.len() - (cols_count - 1);
+ anonymous_col_idx_base += cols_count * (tuples.len() - 1);
write!(
sql,
"SELECT {} FROM (VALUES {})",
- cols(anonymous_col_idx_base),
+ cols(&mut anonymous_col_idx_base),
values
)
.map_err(|e| {
diff --git a/sbroad-core/src/executor/engine/mock.rs b/sbroad-core/src/executor/engine/mock.rs
index 751164ba7d..7e72e75959 100644
--- a/sbroad-core/src/executor/engine/mock.rs
+++ b/sbroad-core/src/executor/engine/mock.rs
@@ -167,6 +167,19 @@ impl RouterConfigurationMock {
Table::new_seg("\"t1\"", columns, sharding_key).unwrap(),
);
+ let columns = vec![
+ Column::new("\"e\"", Type::Unsigned, ColumnRole::User),
+ Column::new("\"f\"", Type::Unsigned, ColumnRole::User),
+ Column::new("\"g\"", Type::Unsigned, ColumnRole::User),
+ Column::new("\"h\"", Type::Unsigned, ColumnRole::User),
+ Column::new("\"bucket_id\"", Type::Unsigned, ColumnRole::Sharding),
+ ];
+ let sharding_key: &[&str] = &["\"e\"", "\"f\""];
+ tables.insert(
+ "\"t2\"".to_string(),
+ Table::new_seg("\"t2\"", columns, sharding_key).unwrap(),
+ );
+
RouterConfigurationMock {
tables,
bucket_count: 10000,
diff --git a/sbroad-core/src/executor/tests.rs b/sbroad-core/src/executor/tests.rs
index f1f54f9a36..a6feabe7e0 100644
--- a/sbroad-core/src/executor/tests.rs
+++ b/sbroad-core/src/executor/tests.rs
@@ -1352,6 +1352,9 @@ mod between;
#[cfg(test)]
mod bucket_id;
+#[cfg(test)]
+mod empty_motion;
+
#[cfg(test)]
mod not_in;
diff --git a/sbroad-core/src/executor/tests/empty_motion.rs b/sbroad-core/src/executor/tests/empty_motion.rs
new file mode 100644
index 0000000000..ab51d06c34
--- /dev/null
+++ b/sbroad-core/src/executor/tests/empty_motion.rs
@@ -0,0 +1,97 @@
+use pretty_assertions::assert_eq;
+
+use crate::backend::sql::ir::PatternWithParams;
+use crate::executor::engine::mock::RouterRuntimeMock;
+use crate::executor::result::ProducerResult;
+use crate::executor::vtable::VirtualTable;
+use crate::ir::relation::{Column, ColumnRole, Type};
+use crate::ir::transformation::redistribution::{DataGeneration, MotionPolicy};
+use crate::ir::value::Value;
+
+use super::*;
+
+#[test]
+fn empty_motion1_test() {
+ let sql = r#"SELECT * FROM (
+ SELECT "t"."a", "t"."b" FROM "t" INNER JOIN "t2" ON "t"."a" = "t2"."g" and "t"."b" = "t2"."h"
+ WHERE "t"."a" = 0
+ EXCEPT
+ SELECT "t"."a", "t"."b" FROM "t" INNER JOIN "t2" ON "t"."a" = "t2"."g" and "t"."b" = "t2"."h"
+ WHERE "t"."a" = 1
+ ) as q"#;
+
+ let coordinator = RouterRuntimeMock::new();
+
+ let mut query = Query::new(&coordinator, sql, vec![]).unwrap();
+ let motion1_id = query.exec_plan.get_ir_plan().clone_slices().unwrap()[0][0];
+ let mut virtual_t1 = t2_empty();
+ if let MotionPolicy::Segment(key) = get_motion_policy(query.exec_plan.get_ir_plan(), motion1_id)
+ {
+ query
+ .reshard_vtable(&mut virtual_t1, key, &DataGeneration::None)
+ .unwrap();
+ }
+ query.coordinator.add_virtual_table(motion1_id, virtual_t1);
+ let motion2_id = query.exec_plan.get_ir_plan().clone_slices().unwrap()[0][1];
+ let mut virtual_t2 = t2_empty();
+ if let MotionPolicy::Segment(key) = get_motion_policy(query.exec_plan.get_ir_plan(), motion2_id)
+ {
+ query
+ .reshard_vtable(&mut virtual_t2, key, &DataGeneration::None)
+ .unwrap();
+ }
+ query.coordinator.add_virtual_table(motion2_id, virtual_t2);
+
+ let result = *query
+ .dispatch()
+ .unwrap()
+ .downcast::<ProducerResult>()
+ .unwrap();
+
+ let mut expected = ProducerResult::new();
+ expected.rows.extend(vec![vec![
+ Value::String("Execute query on all buckets".into()),
+ Value::String(String::from(PatternWithParams::new(
+ format!(
+ "{} {} {} {} {} {} {} {} {} {} {} {} {} {}",
+ r#"SELECT "Q"."a", "Q"."b" FROM"#,
+ r#"(SELECT "t"."a", "t"."b" FROM"#,
+ r#"(SELECT "t"."a", "t"."b", "t"."c", "t"."d" FROM "t") as "t""#,
+ r#"INNER JOIN"#,
+ r#"(SELECT COLUMN_1 as "g",COLUMN_2 as "h" FROM (VALUES (null,null)) WHERE FALSE) as "t2""#,
+ r#"ON ("t"."a", "t"."b") = ("t2"."g", "t2"."h")"#,
+ r#"WHERE ("t"."a") = (?)"#,
+ r#"EXCEPT"#,
+ r#"SELECT "t"."a", "t"."b" FROM"#,
+ r#"(SELECT "t"."a", "t"."b", "t"."c", "t"."d" FROM "t") as "t""#,
+ r#"INNER JOIN"#,
+ r#"(SELECT COLUMN_3 as "g",COLUMN_4 as "h" FROM (VALUES (null,null)) WHERE FALSE) as "t2""#,
+ r#"ON ("t"."a", "t"."b") = ("t2"."g", "t2"."h")"#,
+ r#"WHERE ("t"."a") = (?)) as "Q""#,
+ ),
+ vec![Value::from(0_u64), Value::from(1_u64)],
+ ))),
+ ]]);
+
+ assert_eq!(expected, result);
+}
+
+fn t2_empty() -> VirtualTable {
+ let mut virtual_table = VirtualTable::new();
+
+ virtual_table.add_column(Column {
+ name: "g".into(),
+ r#type: Type::Integer,
+ role: ColumnRole::User,
+ });
+
+ virtual_table.add_column(Column {
+ name: "h".into(),
+ r#type: Type::Integer,
+ role: ColumnRole::User,
+ });
+
+ virtual_table.set_alias("\"t2\"").unwrap();
+
+ virtual_table
+}
--
GitLab