From 0a5a490269dd6bac6045b2645d30e3c61e186da7 Mon Sep 17 00:00:00 2001
From: Arseniy Volynets <a.volynets@picodata.io>
Date: Wed, 2 Oct 2024 10:43:33 +0300
Subject: [PATCH] fix: use Buckets::Any for Values

- Earlier we returned empty buckets for
Values/ValuesRow nodes and then choosed
random bucket for execution if they were
a subtree root.
- Buckets::Any provides the same semantics,
use it instead and simplify `empty_query_result`
function, which now always returns empty result.
---
 sbroad-core/src/executor/bucket.rs              |  5 ++---
 sbroad-core/src/executor/engine/helpers.rs      | 17 ++++-------------
 .../src/executor/engine/helpers/vshard.rs       |  9 +--------
 3 files changed, 7 insertions(+), 24 deletions(-)

diff --git a/sbroad-core/src/executor/bucket.rs b/sbroad-core/src/executor/bucket.rs
index df7742f193..4e385ad7b8 100644
--- a/sbroad-core/src/executor/bucket.rs
+++ b/sbroad-core/src/executor/bucket.rs
@@ -591,9 +591,8 @@ where
                 }
                 Relational::Values(Values { output, .. })
                 | Relational::ValuesRow(ValuesRow { output, .. }) => {
-                    // At the moment values rows are located on the coordinator,
-                    // so there are no buckets to execute on.
-                    self.bucket_map.insert(*output, Buckets::new_empty());
+                    // We can materialize buckets on any node.
+                    self.bucket_map.insert(*output, Buckets::Any);
                 }
                 Relational::SelectWithoutScan(SelectWithoutScan { output, .. }) => {
                     self.bucket_map.insert(*output, Buckets::Any);
diff --git a/sbroad-core/src/executor/engine/helpers.rs b/sbroad-core/src/executor/engine/helpers.rs
index c0a2c8a092..0e6437bb0f 100644
--- a/sbroad-core/src/executor/engine/helpers.rs
+++ b/sbroad-core/src/executor/engine/helpers.rs
@@ -48,7 +48,6 @@ use crate::{
         vtable::{calculate_vtable_unified_types, VTableTuple, VirtualTable},
     },
     ir::{
-        distribution::Distribution,
         relation::{Column, ColumnRole, Type},
         transformation::redistribution::{MotionKey, MotionPolicy},
         tree::Snapshot,
@@ -763,26 +762,20 @@ fn init_sharded_update_tuple_builder(
 pub fn empty_query_result(
     plan: &ExecutionPlan,
     return_format: DispatchReturnFormat,
-) -> Result<Option<Box<dyn Any>>, SbroadError> {
+) -> Result<Box<dyn Any>, SbroadError> {
     let query_type = plan.query_type()?;
     match query_type {
         QueryType::DML => {
             let result = ConsumerResult::default();
             let tuple = Tuple::new(&[result])
                 .map_err(|e| SbroadError::Invalid(Entity::Tuple, Some(format_smolstr!("{e:?}"))))?;
-            Ok(Some(Box::new(tuple) as Box<dyn Any>))
+            Ok(Box::new(tuple) as Box<dyn Any>)
         }
         QueryType::DQL => {
             // Get metadata (column types) from the top node's output tuple.
             let ir_plan = plan.get_ir_plan();
             let top_id = ir_plan.get_top()?;
             let top_output_id = ir_plan.get_relation_node(top_id)?.output();
-            if let Distribution::Segment { .. } = ir_plan.get_distribution(top_output_id)? {
-            } else {
-                // We can have some `select values` query that should be executed on a random
-                // node rather then returning an empty result.
-                return Ok(None);
-            }
             let columns = ir_plan.get_row_list(top_output_id)?;
             let mut metadata = Vec::with_capacity(columns.len());
             for col_id in columns {
@@ -809,7 +802,7 @@ pub fn empty_query_result(
             let tuple = Tuple::new(&[result])
                 .map_err(|e| SbroadError::Invalid(Entity::Tuple, Some(format_smolstr!("{e:?}"))))?;
             let res = tuple.convert(return_format)?;
-            Ok(Some(res))
+            Ok(res)
         }
     }
 }
@@ -890,9 +883,7 @@ pub fn dispatch_impl(
 
     debug!(Option::from("dispatch"), &format!("sub plan: {sub_plan:?}"));
     if has_zero_limit_clause(&sub_plan)? {
-        if let Some(result) = empty_query_result(&sub_plan, return_format.clone())? {
-            return Ok(result);
-        }
+        return empty_query_result(&sub_plan, return_format.clone());
     }
     dispatch_by_buckets(sub_plan, buckets, &tier_runtime, return_format)
 }
diff --git a/sbroad-core/src/executor/engine/helpers/vshard.rs b/sbroad-core/src/executor/engine/helpers/vshard.rs
index 43aa83a90b..2d96e146a1 100644
--- a/sbroad-core/src/executor/engine/helpers/vshard.rs
+++ b/sbroad-core/src/executor/engine/helpers/vshard.rs
@@ -704,16 +704,9 @@ pub fn impl_exec_ir_on_buckets(
     waiting_timeout: u64,
     tier_name: Option<&SmolStr>,
 ) -> Result<Box<dyn Any>, SbroadError> {
-    let mut buckets = buckets;
-    let random_bucket = runtime.get_random_bucket();
     if let Buckets::Filtered(buckets_set) = buckets {
         if buckets_set.is_empty() {
-            match empty_query_result(&sub_plan, return_format.clone())? {
-                Some(res) => return Ok(res),
-                None => {
-                    buckets = &random_bucket;
-                }
-            }
+            return empty_query_result(&sub_plan, return_format.clone());
         }
     }
 
-- 
GitLab