From ca1595b98faa2e5eeb18757bdd0e11b14b980251 Mon Sep 17 00:00:00 2001
From: EmirVildanov <reddog201030@gmail.com>
Date: Tue, 17 Sep 2024 13:31:19 +0300
Subject: [PATCH] fix: support case of several Relations referencing the same
SubQuery in `take_subtree`
---
sbroad-core/src/executor/ir.rs | 29 +++++++++++++++++--
.../transformation/redistribution/groupby.rs | 8 +++++
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/sbroad-core/src/executor/ir.rs b/sbroad-core/src/executor/ir.rs
index 6e02205496..36125f1ddc 100644
--- a/sbroad-core/src/executor/ir.rs
+++ b/sbroad-core/src/executor/ir.rs
@@ -441,6 +441,30 @@ impl ExecutionPlan {
}
}
+ // TODO: See note in `add_local_projection` function about SubQueries
+ // to understand why we don't want to replace SubQueries with stubs sometimes.
+ // The reason is that several relational nodes may point to it as a child.
+ let sqs = nodes
+ .iter()
+ .map(|LevelNode(_, id)| *id)
+ .filter(|id| {
+ matches!(
+ plan.get_node(*id),
+ Ok(Node::Relational(Relational::ScanSubQuery(_)))
+ )
+ })
+ .collect::<Vec<_>>();
+ let mut sq_ids: AHashSet<NodeId> = AHashSet::new();
+ for sq_id in sqs {
+ let mut sq_subtree = PostOrder::with_capacity(
+ |node| plan.exec_plan_subtree_iter(node),
+ plan.nodes.len(),
+ );
+ for LevelNode(_, id) in sq_subtree.iter(sq_id) {
+ sq_ids.insert(id);
+ }
+ }
+
let mut subtree_map = SubtreeMap::with_capacity(nodes.len());
let vtables_capacity = self.get_vtables().map_or_else(|| 1, HashMap::len);
// Map of { plan node_id -> virtual table }.
@@ -465,8 +489,9 @@ impl ExecutionPlan {
let mut_plan = self.get_mut_ir_plan();
// Replace the node with some invalid value.
- let mut node: NodeOwned = if cte_ids.contains(&node_id) {
- mut_plan.get_node(node_id)?.get_common_node()
+ let node = mut_plan.get_node(node_id)?;
+ let mut node: NodeOwned = if cte_ids.contains(&node_id) || sq_ids.contains(&node_id) {
+ node.into_owned()
} else {
mut_plan.replace_with_stub(node_id)
};
diff --git a/sbroad-core/src/ir/transformation/redistribution/groupby.rs b/sbroad-core/src/ir/transformation/redistribution/groupby.rs
index 5c713c747b..c377275d64 100644
--- a/sbroad-core/src/ir/transformation/redistribution/groupby.rs
+++ b/sbroad-core/src/ir/transformation/redistribution/groupby.rs
@@ -1084,6 +1084,14 @@ impl Plan {
if matches!(rel_node, Relational::ScanSubQuery { .. })
&& self.is_additional_child(ref_rel_node_id)?
{
+ // Note: For queries like `select sum((VALUES (1))) from t` it may be a problem
+ // that we don't make a copy of a SubQuery node, but just copy its id so that
+ // several Relational node consider it to be a child.
+ // It may be a problem on a `take_subtree` call for the query above:
+ // * `take_subtree` will replace SQ with Invalid while traversing Local(Map)
+ // stage
+ // * Later Final(Reduce) stage won't see the SubQuery.
+ // TODO: Solution would be to use `SubtreeCloner::clone`?
children.push(ref_rel_node_id);
}
}
--
GitLab