From d0a513533f2be187f9fa762160fbefaa532fe7b6 Mon Sep 17 00:00:00 2001
From: EmirVildanov <reddog201030@gmail.com>
Date: Wed, 21 Aug 2024 13:47:50 +0300
Subject: [PATCH] feat: change is_err check for motion vtables with new
`contains` function
---
sbroad-core/src/executor/bucket.rs | 3 ++-
sbroad-core/src/executor/engine/helpers.rs | 19 ++++++++-----------
sbroad-core/src/executor/ir.rs | 7 ++++++-
3 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/sbroad-core/src/executor/bucket.rs b/sbroad-core/src/executor/bucket.rs
index 478761d66..b7a4e09ea 100644
--- a/sbroad-core/src/executor/bucket.rs
+++ b/sbroad-core/src/executor/bucket.rs
@@ -351,7 +351,8 @@ where
MotionPolicy::LocalSegment(_) => {
// See `dispatch` method in `src/executor.rs` in order to understand when
// `virtual_table` materialized and when not for LocalSegment.
- if let Ok(virtual_table) = self.exec_plan.get_motion_vtable(node_id) {
+ if self.exec_plan.contains_vtable_for_motion(node_id) {
+ let virtual_table = self.exec_plan.get_motion_vtable(node_id)?;
// In a case of `insert .. values ..` it is possible built a local
// segmented virtual table right on the router. So, we can use its
// buckets from the index to determine the buckets for the insert.
diff --git a/sbroad-core/src/executor/engine/helpers.rs b/sbroad-core/src/executor/engine/helpers.rs
index 1d7cbcd5a..366e35216 100644
--- a/sbroad-core/src/executor/engine/helpers.rs
+++ b/sbroad-core/src/executor/engine/helpers.rs
@@ -224,7 +224,7 @@ pub fn build_optional_binary(mut exec_plan: ExecutionPlan) -> Result<Binary, Sbr
// SQL is needed only for the motion node subtree.
// HACK: we don't actually need SQL when the subtree is already
// materialized into a virtual table on the router.
- let already_materialized = exec_plan.get_motion_vtable(motion_id).is_ok();
+ let already_materialized = exec_plan.contains_vtable_for_motion(motion_id);
if already_materialized {
OrderedSyntaxNodes::empty()
@@ -935,7 +935,7 @@ pub fn dispatch_by_buckets(
if !vtable.get_bucket_index().is_empty() {
return Err(SbroadError::Invalid(
Entity::Motion,
- Some(format_smolstr!("motion ({motion_id:?}) in subtree with distribution Single, but policy is not Full!")),
+ Some(format_smolstr!("Motion ({motion_id:?}) in subtree with distribution Single, but policy is not Full.")),
));
}
}
@@ -1656,10 +1656,9 @@ where
let space_name = plan.dml_node_table(update_id)?.name().clone();
let mut result = ConsumerResult::default();
let is_sharded = plan.is_sharded_update(update_id)?;
- let build_vtable_locally = optional
+ let build_vtable_locally = !optional
.exec_plan
- .get_motion_vtable(update_child_id)
- .is_err();
+ .contains_vtable_for_motion(update_child_id);
if build_vtable_locally {
// it is relevant only for local Update.
if is_sharded {
@@ -1938,10 +1937,9 @@ where
let builder = init_delete_tuple_builder(plan, delete_id)?;
let space_name = plan.dml_node_table(delete_id)?.name().clone();
let mut result = ConsumerResult::default();
- let build_vtable_locally = optional
+ let build_vtable_locally = !optional
.exec_plan
- .get_motion_vtable(delete_child_id)
- .is_err();
+ .contains_vtable_for_motion(delete_child_id);
if build_vtable_locally {
materialize_vtable_locally(runtime, optional, required, delete_child_id)?;
}
@@ -1996,10 +1994,9 @@ where
// The same for `UPDATE`.
// Check is we need to execute an SQL subtree (case 1).
- let build_vtable_locally = optional
+ let build_vtable_locally = !optional
.exec_plan
- .get_motion_vtable(insert_child_id)
- .is_err();
+ .contains_vtable_for_motion(insert_child_id);
if build_vtable_locally {
materialize_vtable_locally(runtime, optional, required, insert_child_id)?;
}
diff --git a/sbroad-core/src/executor/ir.rs b/sbroad-core/src/executor/ir.rs
index 79280ba12..157c251e0 100644
--- a/sbroad-core/src/executor/ir.rs
+++ b/sbroad-core/src/executor/ir.rs
@@ -130,6 +130,11 @@ impl ExecutionPlan {
self.vtables = Some(VirtualTableMap::new(vtables));
}
+ pub fn contains_vtable_for_motion(&self, motion_id: usize) -> bool {
+ self.get_vtables()
+ .map_or(false, |map| map.contains_key(&motion_id))
+ }
+
/// Get motion virtual table
pub fn get_motion_vtable(&self, motion_id: NodeId) -> Result<Rc<VirtualTable>, SbroadError> {
if let Some(vtable) = self.get_vtables() {
@@ -309,7 +314,7 @@ impl ExecutionPlan {
Relational::Motion { .. } | Relational::Insert { .. } | Relational::Delete { .. } => {
Err(SbroadError::Invalid(
Entity::Relational,
- Some("invalid motion child node".to_smolstr()),
+ Some(format_smolstr!("invalid motion child node: {rel:?}.")),
))
}
}
--
GitLab