From 5553b0c1a45f003974ce91a5a0a03c39e0a8aa06 Mon Sep 17 00:00:00 2001
From: Denis Smirnov <sd@picodata.io>
Date: Mon, 31 Jan 2022 17:05:15 +0700
Subject: [PATCH] fix: resolve conflicts on rebase
---
src/ir/distribution.rs | 56 +++++++++----------
src/ir/expression.rs | 31 +++++-----
src/ir/expression/tests.rs | 8 +--
src/ir/transformation/redistribution.rs | 51 +++++++----------
src/ir/transformation/redistribution/tests.rs | 11 ++--
.../full_motion_less_for_sub_query.yaml | 6 ++
...otion_non_segment_outer_for_sub_query.yaml | 6 ++
.../redistribution/local_sub_query.yaml | 6 ++
.../redistribution/multiple_sub_queries.yaml | 14 ++++-
.../segment_motion_for_sub_query.yaml | 6 ++
10 files changed, 109 insertions(+), 86 deletions(-)
diff --git a/src/ir/distribution.rs b/src/ir/distribution.rs
index c5bc6cb89a..a5b59b15c6 100644
--- a/src/ir/distribution.rs
+++ b/src/ir/distribution.rs
@@ -122,38 +122,36 @@ impl Plan {
..
}) = self.get_node(node_id)?
{
- // Get a relational node, containing current row.
- parent_node = Some(*id_map.get(parent).ok_or(QueryPlannerError::InvalidNode)?);
- if let Node::Relational(relational_op) =
- self.get_node(parent_node.ok_or(QueryPlannerError::InvalidNode)?)?
- {
- if let Some(children) = relational_op.children() {
- // References in the branch node.
- let child_pos_list: &Vec<usize> = targets
- .as_ref()
- .ok_or(QueryPlannerError::InvalidReference)?;
- for target in child_pos_list {
- let child_node: usize = *children
- .get(*target)
- .ok_or(QueryPlannerError::ValueOutOfRange)?;
- child_set.insert(child_node);
- child_pos_map.insert((child_node, *position), pos);
- }
- } else {
- // References in the leaf (relation scan) node.
- if targets.is_some() {
- return Err(QueryPlannerError::InvalidReference);
- }
- if let Relational::ScanRelation { relation, .. } = relational_op {
- table_set.insert(relation.clone());
- table_pos_map.insert(*position, pos);
- } else {
- return Err(QueryPlannerError::InvalidReference);
- }
+ parent_node = Some(self.get_map_relational_value(*parent)?);
+ let relational_op =
+ self.get_relation_node(parent_node.ok_or(QueryPlannerError::InvalidRelation)?)?;
+
+ if let Some(children) = relational_op.children() {
+ // References in the branch node.
+ let child_pos_list: &Vec<usize> = targets
+ .as_ref()
+ .ok_or(QueryPlannerError::InvalidReference)?;
+ for target in child_pos_list {
+ let child_node: usize = *children
+ .get(*target)
+ .ok_or(QueryPlannerError::ValueOutOfRange)?;
+ child_set.insert(child_node);
+ child_pos_map.insert((child_node, *position), pos);
}
} else {
- return Err(QueryPlannerError::InvalidNode);
+ // References in the leaf (relation scan) node.
+ if targets.is_some() {
+ return Err(QueryPlannerError::InvalidReference);
+ }
+ if let Relational::ScanRelation { relation, .. } = relational_op {
+ table_set.insert(relation.clone());
+ table_pos_map.insert(*position, pos);
+ } else {
+ return Err(QueryPlannerError::InvalidReference);
+ }
}
+ } else {
+ return Err(QueryPlannerError::InvalidNode);
}
Ok(())
};
diff --git a/src/ir/expression.rs b/src/ir/expression.rs
index e3cb4ea1fd..1a5f0a0094 100644
--- a/src/ir/expression.rs
+++ b/src/ir/expression.rs
@@ -11,7 +11,7 @@ use super::value::Value;
use super::{Node, Nodes, Plan};
use crate::errors::QueryPlannerError;
use serde::{Deserialize, Serialize};
-use std::collections::{HashMap, HashSet};
+use std::collections::HashSet;
use traversal::DftPost;
/// Tuple tree build blocks.
@@ -507,30 +507,35 @@ impl Plan {
///
/// # Errors
/// - reference is invalid
+ /// - `relational_map` is not initialized
pub fn get_relational_from_reference_node(
&self,
ref_id: usize,
- map: &HashMap<usize, usize>,
) -> Result<HashSet<usize>, QueryPlannerError> {
let mut rel_nodes: HashSet<usize> = HashSet::new();
+ if self.relational_map.is_none() {
+ return Err(QueryPlannerError::CustomError(String::from(
+ "Initialized relational map in the plan",
+ )));
+ }
+
if let Node::Expression(Expression::Reference {
targets, parent, ..
}) = self.get_node(ref_id)?
{
- if let Some(rel_id) = map.get(parent) {
- if let Node::Relational(rel) = self.get_node(*rel_id)? {
- if let Some(children) = rel.children() {
- if let Some(positions) = targets {
- for pos in positions {
- if let Some(child) = children.get(*pos) {
- rel_nodes.insert(*child);
- }
+ let referred_rel_id = self.get_map_relational_value(*parent)?;
+ if let Node::Relational(rel) = self.get_node(referred_rel_id)? {
+ if let Some(children) = rel.children() {
+ if let Some(positions) = targets {
+ for pos in positions {
+ if let Some(child) = children.get(*pos) {
+ rel_nodes.insert(*child);
}
}
}
- return Ok(rel_nodes);
}
+ return Ok(rel_nodes);
}
}
Err(QueryPlannerError::InvalidReference)
@@ -541,10 +546,10 @@ impl Plan {
/// # Errors
/// - node is not row
/// - row is invalid
+ /// - `relational_map` is not initialized
pub fn get_relational_from_row_nodes(
&self,
row_id: usize,
- map: &HashMap<usize, usize>,
) -> Result<HashSet<usize>, QueryPlannerError> {
let mut rel_nodes: HashSet<usize> = HashSet::new();
@@ -552,7 +557,7 @@ impl Plan {
let post_tree = DftPost::new(&row_id, |node| self.nodes.expr_iter(node, false));
for (_, node) in post_tree {
if let Node::Expression(Expression::Reference { .. }) = self.get_node(*node)? {
- rel_nodes.extend(&self.get_relational_from_reference_node(*node, map)?);
+ rel_nodes.extend(&self.get_relational_from_reference_node(*node)?);
}
}
return Ok(rel_nodes);
diff --git a/src/ir/expression/tests.rs b/src/ir/expression/tests.rs
index 1bd82ad18b..c275dda323 100644
--- a/src/ir/expression/tests.rs
+++ b/src/ir/expression/tests.rs
@@ -30,8 +30,8 @@ fn rel_nodes_from_reference_in_scan() {
let scan_id = plan.add_scan("t").unwrap();
let output = plan.get_relational_output(scan_id).unwrap();
- let map = plan.relational_id_map();
- let rel_set = plan.get_relational_from_row_nodes(output, &map).unwrap();
+ plan.build_relational_map();
+ let rel_set = plan.get_relational_from_row_nodes(output).unwrap();
assert_eq!(true, rel_set.is_empty());
}
@@ -47,8 +47,8 @@ fn rel_nodes_from_reference_in_proj() {
let proj_id = plan.add_proj(scan_id, &["a"]).unwrap();
let output = plan.get_relational_output(proj_id).unwrap();
- let map = plan.relational_id_map();
- let rel_set = plan.get_relational_from_row_nodes(output, &map).unwrap();
+ plan.build_relational_map();
+ let rel_set = plan.get_relational_from_row_nodes(output).unwrap();
assert_eq!(1, rel_set.len());
assert_eq!(Some(&scan_id), rel_set.get(&scan_id));
}
diff --git a/src/ir/transformation/redistribution.rs b/src/ir/transformation/redistribution.rs
index bde7ab382e..0748c98fd1 100644
--- a/src/ir/transformation/redistribution.rs
+++ b/src/ir/transformation/redistribution.rs
@@ -28,21 +28,17 @@ struct BoolOp {
impl BoolOp {
fn from_expr(plan: &Plan, expr_id: usize) -> Result<Self, QueryPlannerError> {
- if let Node::Expression(expr) = plan.get_node(expr_id)? {
- if let Expression::Bool {
- left, op, right, ..
- } = expr
- {
- Ok(BoolOp {
- left: *left,
- op: op.clone(),
- right: *right,
- })
- } else {
- Err(QueryPlannerError::InvalidBool)
- }
+ if let Expression::Bool {
+ left, op, right, ..
+ } = plan.get_expression_node(expr_id)?
+ {
+ Ok(BoolOp {
+ left: *left,
+ op: op.clone(),
+ right: *right,
+ })
} else {
- Err(QueryPlannerError::InvalidNode)
+ Err(QueryPlannerError::InvalidBool)
}
}
}
@@ -97,13 +93,13 @@ impl Plan {
///
/// # Errors
/// - reference points to multiple sub-queries (invalid SQL)
+ /// - `relational_map` is not initialized
fn get_sub_query_from_row_node(
&self,
ref_id: usize,
- map: &HashMap<usize, usize>,
) -> Result<Option<usize>, QueryPlannerError> {
let mut sq_set: HashSet<usize> = HashSet::new();
- let rel_nodes = self.get_relational_from_row_nodes(ref_id, map)?;
+ let rel_nodes = self.get_relational_from_row_nodes(ref_id)?;
for rel_id in rel_nodes {
if let Node::Relational(Relational::ScanSubQuery { .. }) = self.get_node(rel_id)? {
sq_set.insert(rel_id);
@@ -209,20 +205,19 @@ impl Plan {
fn resolve_sub_query_conflicts(
&mut self,
expr_id: usize,
- map: &HashMap<usize, usize>,
) -> Result<HashMap<usize, MotionPolicy>, QueryPlannerError> {
let nodes = self.get_bool_nodes_with_row_children(expr_id)?;
for node in &nodes {
let bool_op = BoolOp::from_expr(self, *node)?;
- self.set_distribution(bool_op.left, map)?;
- self.set_distribution(bool_op.right, map)?;
+ self.set_distribution(bool_op.left)?;
+ self.set_distribution(bool_op.right)?;
}
let mut strategy: HashMap<usize, MotionPolicy> = HashMap::new();
for node in &nodes {
let bool_op = BoolOp::from_expr(self, *node)?;
- let left = self.get_sub_query_from_row_node(bool_op.left, map)?;
- let right = self.get_sub_query_from_row_node(bool_op.right, map)?;
+ let left = self.get_sub_query_from_row_node(bool_op.left)?;
+ let right = self.get_sub_query_from_row_node(bool_op.right)?;
match left {
Some(left_sq) => {
match right {
@@ -269,22 +264,18 @@ impl Plan {
/// - failed to resolve distribution conflicts
/// - failed to set distribution
pub fn add_motions(&mut self) -> Result<(), QueryPlannerError> {
- let map = self.relational_id_map();
+ self.build_relational_map();
let nodes = self.get_relational_nodes_dfs_post()?;
for id in &nodes {
- let rel_op: Relational = match self.get_node(*id)? {
- Node::Expression(_) => return Err(QueryPlannerError::InvalidNode),
- Node::Relational(rel) => rel.clone(),
- };
- match rel_op {
+ match self.get_relation_node(*id)?.clone() {
// At the moment our grammar and IR constructor
// don't support projection with sub queries.
Relational::Projection { output, .. }
| Relational::ScanRelation { output, .. }
| Relational::ScanSubQuery { output, .. }
| Relational::UnionAll { output, .. } => {
- self.set_distribution(output, &map)?;
+ self.set_distribution(output)?;
}
Relational::Motion { .. } => {
// We can apply this transformation only once,
@@ -294,9 +285,9 @@ impl Plan {
)));
}
Relational::Selection { output, filter, .. } => {
- let strategy = self.resolve_sub_query_conflicts(filter, &map)?;
+ let strategy = self.resolve_sub_query_conflicts(filter)?;
self.create_motion_nodes(*id, &strategy)?;
- self.set_distribution(output, &map)?;
+ self.set_distribution(output)?;
}
Relational::InnerJoin { .. } => {
// TODO: resolve join conflicts and set the output row distribution
diff --git a/src/ir/transformation/redistribution/tests.rs b/src/ir/transformation/redistribution/tests.rs
index d949ee4f42..b59d63f73b 100644
--- a/src/ir/transformation/redistribution/tests.rs
+++ b/src/ir/transformation/redistribution/tests.rs
@@ -44,22 +44,19 @@ fn segment_motion_for_sub_query() {
let select_id = plan.add_select(&children[..], eq_id, id).unwrap();
plan.set_top(select_id).unwrap();
- let map = plan.relational_id_map();
+ plan.build_relational_map();
let mut expected_rel_set: HashSet<usize> = HashSet::new();
expected_rel_set.insert(sq_id);
assert_eq!(
expected_rel_set,
- plan.get_relational_from_row_nodes(b_id, &map).unwrap()
- );
- assert_eq!(
- Some(sq_id),
- plan.get_sub_query_from_row_node(b_id, &map).unwrap()
+ plan.get_relational_from_row_nodes(b_id).unwrap()
);
+ assert_eq!(Some(sq_id), plan.get_sub_query_from_row_node(b_id).unwrap());
assert_eq!(
QueryPlannerError::UninitializedDistribution,
- plan.resolve_sub_query_conflicts(eq_id, &map).unwrap_err()
+ plan.resolve_sub_query_conflicts(eq_id).unwrap_err()
);
plan.add_motions().unwrap();
diff --git a/tests/artifactory/ir/transformation/redistribution/full_motion_less_for_sub_query.yaml b/tests/artifactory/ir/transformation/redistribution/full_motion_less_for_sub_query.yaml
index d82f83c30b..00420543e6 100644
--- a/tests/artifactory/ir/transformation/redistribution/full_motion_less_for_sub_query.yaml
+++ b/tests/artifactory/ir/transformation/redistribution/full_motion_less_for_sub_query.yaml
@@ -201,6 +201,12 @@ relations:
positions:
- 0
name: t1
+relational_map:
+ 0: 3
+ 10: 13
+ 4: 9
+ 14: 17
+ 18: 26
slices:
-
- 30
diff --git a/tests/artifactory/ir/transformation/redistribution/full_motion_non_segment_outer_for_sub_query.yaml b/tests/artifactory/ir/transformation/redistribution/full_motion_non_segment_outer_for_sub_query.yaml
index 6afe1446c5..da7e3014ba 100644
--- a/tests/artifactory/ir/transformation/redistribution/full_motion_non_segment_outer_for_sub_query.yaml
+++ b/tests/artifactory/ir/transformation/redistribution/full_motion_non_segment_outer_for_sub_query.yaml
@@ -220,6 +220,12 @@ relations:
positions:
- 0
name: t2
+relational_map:
+ 0: 5
+ 10: 13
+ 14: 17
+ 18: 28
+ 6: 9
slices:
-
- 32
diff --git a/tests/artifactory/ir/transformation/redistribution/local_sub_query.yaml b/tests/artifactory/ir/transformation/redistribution/local_sub_query.yaml
index e296537760..df8921440f 100644
--- a/tests/artifactory/ir/transformation/redistribution/local_sub_query.yaml
+++ b/tests/artifactory/ir/transformation/redistribution/local_sub_query.yaml
@@ -179,5 +179,11 @@ relations:
positions:
- 0
name: t2
+relational_map:
+ 8: 11
+ 0: 3
+ 4: 7
+ 12: 15
+ 16: 24
slices: ~
top: 24
diff --git a/tests/artifactory/ir/transformation/redistribution/multiple_sub_queries.yaml b/tests/artifactory/ir/transformation/redistribution/multiple_sub_queries.yaml
index a55d631166..48250d984a 100644
--- a/tests/artifactory/ir/transformation/redistribution/multiple_sub_queries.yaml
+++ b/tests/artifactory/ir/transformation/redistribution/multiple_sub_queries.yaml
@@ -351,8 +351,16 @@ relations:
positions:
- 0
name: t2
-slices:
- -
- - 50
+relational_map:
+ 28: 31
+ 24: 27
+ 14: 17
+ 32: 46
+ 10: 13
+ 4: 9
+ 0: 3
+ 18: 23
+slices:
+ - - 50
- 54
top: 46
diff --git a/tests/artifactory/ir/transformation/redistribution/segment_motion_for_sub_query.yaml b/tests/artifactory/ir/transformation/redistribution/segment_motion_for_sub_query.yaml
index 6eae3d01b8..614dee176f 100644
--- a/tests/artifactory/ir/transformation/redistribution/segment_motion_for_sub_query.yaml
+++ b/tests/artifactory/ir/transformation/redistribution/segment_motion_for_sub_query.yaml
@@ -207,6 +207,12 @@ relations:
positions:
- 0
name: t2
+relational_map:
+ 18: 26
+ 10: 13
+ 0: 3
+ 4: 9
+ 14: 17
slices:
-
- 30
--
GitLab