From b2a27ffc725daa96fbf98d8317716bc4621d25d3 Mon Sep 17 00:00:00 2001
From: Denis Smirnov <sd@picodata.io>
Date: Thu, 20 Oct 2022 21:26:23 +0700
Subject: [PATCH] fix: bug in the join condition
We have already fixed the problems with the bucket_id system column
(fc8b6d9367b57ec0c209928b7b03e8a79232086f), but forgot to take care
of the join condition references. Now this problem is fixed.
---
sbroad-core/src/executor/engine/mock.rs | 11 +++++
sbroad-core/src/executor/tests/frontend.rs | 28 +++++++++++++
sbroad-core/src/ir/operator.rs | 49 +++++++++++++++++++---
3 files changed, 82 insertions(+), 6 deletions(-)
diff --git a/sbroad-core/src/executor/engine/mock.rs b/sbroad-core/src/executor/engine/mock.rs
index 1b9496617b..b49c677b77 100644
--- a/sbroad-core/src/executor/engine/mock.rs
+++ b/sbroad-core/src/executor/engine/mock.rs
@@ -198,6 +198,17 @@ impl RouterConfigurationMock {
Table::new_seg("\"t2\"", columns, sharding_key).unwrap(),
);
+ let columns = vec![
+ Column::new("\"bucket_id\"", Type::Unsigned, ColumnRole::Sharding),
+ Column::new("\"a\"", Type::String, ColumnRole::User),
+ Column::new("\"b\"", Type::Integer, ColumnRole::User),
+ ];
+ let sharding_key: &[&str] = &["\"a\""];
+ tables.insert(
+ "\"t3\"".to_string(),
+ Table::new_seg("\"t3\"", columns, sharding_key).unwrap(),
+ );
+
RouterConfigurationMock {
functions,
tables,
diff --git a/sbroad-core/src/executor/tests/frontend.rs b/sbroad-core/src/executor/tests/frontend.rs
index 9fd5f9c1c5..c80b30ce7b 100644
--- a/sbroad-core/src/executor/tests/frontend.rs
+++ b/sbroad-core/src/executor/tests/frontend.rs
@@ -115,3 +115,31 @@ fn front_explain_select_sql2() {
panic!("Explain must be string")
}
}
+
+#[test]
+fn front_explain_select_sql3() {
+ let sql = r#"EXPLAIN SELECT "a" FROM "t3" as "q1"
+ INNER JOIN (SELECT "t3"."a" as "a2", "t3"."b" as "b2" FROM "t3") as "q2"
+ on "q1"."a" = "q2"."a2""#;
+
+ let metadata = &RouterRuntimeMock::new();
+ let mut query = Query::new(metadata, sql, vec![]).unwrap();
+
+ let expected_explain = format!(
+ "{}\n{}\n{}\n{}\n{}\n{}\n{}\n{}\n",
+ r#"projection ("q1"."a" -> "a")"#,
+ r#" join on ROW("q1"."a") = ROW("q2"."a2")"#,
+ r#" scan "q1""#,
+ r#" projection ("q1"."a" -> "a", "q1"."b" -> "b")"#,
+ r#" scan "t3" -> "q1""#,
+ r#" scan "q2""#,
+ r#" projection ("t3"."a" -> "a2", "t3"."b" -> "b2")"#,
+ r#" scan "t3""#,
+ );
+
+ if let Ok(actual_explain) = query.dispatch().unwrap().downcast::<String>() {
+ assert_eq!(expected_explain, *actual_explain);
+ } else {
+ panic!("Explain must be string")
+ }
+}
diff --git a/sbroad-core/src/ir/operator.rs b/sbroad-core/src/ir/operator.rs
index dad8c6faec..b438b6967b 100644
--- a/sbroad-core/src/ir/operator.rs
+++ b/sbroad-core/src/ir/operator.rs
@@ -404,7 +404,10 @@ impl Relational {
let output_row = plan.get_expression_node(self.output())?;
let list = output_row.get_row_list()?;
let col_id = *list.get(position).ok_or_else(|| {
- QueryPlannerError::CustomError(String::from("Row has no such alias"))
+ QueryPlannerError::CustomError(format!(
+ "Row doesn't has a column at position {}",
+ position
+ ))
})?;
let col_node = plan.get_expression_node(col_id)?;
if let Expression::Alias { child, .. } = col_node {
@@ -630,24 +633,58 @@ impl Plan {
// remove a sharding column from its output with a
// projection node and wrap the result with a sub-query
// scan.
+ // As a side effect, we also need to update the references
+ // to the child's output in the condition expression as
+ // we have filtered out the sharding column.
let mut children: Vec<usize> = Vec::with_capacity(2);
for child in &[left, right] {
let child_node = self.get_relation_node(*child)?;
- let chid_id = if let Relational::ScanRelation {
+ if let Relational::ScanRelation {
relation, alias, ..
} = child_node
{
+ // We'll need it later to update the condition expression (borrow checker).
+ let table = self.get_relation(relation).ok_or_else(|| {
+ QueryPlannerError::CustomError(format!(
+ "Failed to find relation {} in the plan",
+ relation
+ ))
+ })?;
+ let sharding_column_pos = table.get_bucket_id_position()?;
+
+ // Wrap relation with sub-query scan.
let scan_name = if let Some(alias_name) = alias {
alias_name.clone()
} else {
relation.clone()
};
let proj_id = self.add_proj(*child, &[])?;
- self.add_sub_query(proj_id, Some(&scan_name))?
+ let sq_id = self.add_sub_query(proj_id, Some(&scan_name))?;
+ children.push(sq_id);
+
+ // Update references to the sub-query's output in the condition.
+ let condition_iter = Bft::new(&condition, |node| self.nodes.expr_iter(node, false));
+ let refs = condition_iter
+ .filter_map(|(_, id)| {
+ let expr = self.get_expression_node(*id).ok();
+ if let Some(Expression::Reference { .. }) = expr {
+ Some(*id)
+ } else {
+ None
+ }
+ })
+ .collect::<Vec<_>>();
+ for ref_id in refs {
+ let expr = self.get_mut_expression_node(ref_id)?;
+ if let Expression::Reference { position, .. } = expr {
+ if *position > sharding_column_pos {
+ *position -= 1;
+ }
+ }
+ }
} else {
- *child
- };
- children.push(chid_id);
+ children.push(*child);
+ }
}
if let (Some(left_id), Some(right_id)) = (children.first(), children.get(1)) {
let output = self.add_row_for_join(*left_id, *right_id)?;
--
GitLab