From b4c503be40fc948a4072302ac150b2f1159bd5ec Mon Sep 17 00:00:00 2001
From: Arseniy Volynets <vol0ncar@yandex.ru>
Date: Mon, 20 Mar 2023 22:12:13 +0300
Subject: [PATCH] fix: use MotionPolicy::Full for <>, not in
---
.../test/integration/operators_test.lua | 55 +++++++++++++++++++
.../src/frontend/sql/ir/tests/params.rs | 17 +++---
.../src/ir/transformation/redistribution.rs | 11 +++-
.../redistribution/tests/not_in.rs | 34 ++----------
4 files changed, 77 insertions(+), 40 deletions(-)
diff --git a/sbroad-cartridge/test_app/test/integration/operators_test.lua b/sbroad-cartridge/test_app/test/integration/operators_test.lua
index 1dd7b87a73..3225c6e25a 100644
--- a/sbroad-cartridge/test_app/test/integration/operators_test.lua
+++ b/sbroad-cartridge/test_app/test/integration/operators_test.lua
@@ -132,6 +132,61 @@ g.test_not_eq = function()
})
end
+g.test_not_eq2 = function()
+ -- "t" contains:
+ -- [1, 4.2]
+ -- [2, decimal(6.66)]
+ -- [3, 0.0]
+ -- [4, 0.0]
+ local api = cluster:server("api-1").net_box
+ local r, err = api:call("sbroad.execute", {
+ [[insert into "t" ("id", "a") values (?, ?), (?, ?)]],
+ {3, 0.0, 4, 0.0}
+ })
+ t.assert_equals(err, nil)
+ t.assert_equals(r, {row_count = 2})
+
+ -- make sure table is located on both storages, not only on one storage
+ local storage1 = cluster:server("storage-1-1").net_box
+ r, err = storage1:call("box.execute", {
+ [[select * from "t"]], {}
+ })
+ t.assert_equals(err, nil)
+ t.assert_equals(true, next(r.rows) ~= nil)
+
+ local storage2 = cluster:server("storage-2-1").net_box
+ r, err = storage2:call("box.execute", {
+ [[select * from "t"]], {}
+ })
+ t.assert_equals(err, nil)
+ t.assert_equals(true, next(r.rows) ~= nil)
+
+ r, err = api:call(
+ "sbroad.execute",
+ {
+ [[
+ SELECT "id", u FROM "t" join
+ (select "id" as u from "t") as q
+ on "t"."id" <> q.u
+ ]],
+ {}
+ }
+ )
+ t.assert_equals(err, nil)
+ t.assert_items_equals(r, {
+ metadata = {
+ {name = "t.id", type = "integer"},
+ {name = "Q.U", type = "integer"}
+ },
+ rows = {
+ {1, 2}, {1, 3}, {1, 4},
+ {2, 1}, {2, 3}, {2, 4},
+ {3, 1}, {3, 2}, {3, 4},
+ {4, 1}, {4, 2}, {4, 3},
+ },
+ })
+end
+
g.test_simple_shard_key_union_query = function()
local api = cluster:server("api-1").net_box
diff --git a/sbroad-core/src/frontend/sql/ir/tests/params.rs b/sbroad-core/src/frontend/sql/ir/tests/params.rs
index 336ffe1fc0..f371128c72 100644
--- a/sbroad-core/src/frontend/sql/ir/tests/params.rs
+++ b/sbroad-core/src/frontend/sql/ir/tests/params.rs
@@ -126,14 +126,15 @@ fn front_params6() {
selection ROW("test_space"."sys_op") = ROW(0) or ROW("test_space"."id") not in ROW($0)
scan "test_space"
subquery $0:
-scan
- union all
- projection ("test_space"."id" -> "id")
- selection ROW("test_space"."sys_op") = ROW(1)
- scan "test_space"
- projection ("test_space"."id" -> "id")
- selection ROW("test_space"."sys_op") = ROW(2)
- scan "test_space"
+motion [policy: full]
+ scan
+ union all
+ projection ("test_space"."id" -> "id")
+ selection ROW("test_space"."sys_op") = ROW(1)
+ scan "test_space"
+ projection ("test_space"."id" -> "id")
+ selection ROW("test_space"."sys_op") = ROW(2)
+ scan "test_space"
"#,
);
diff --git a/sbroad-core/src/ir/transformation/redistribution.rs b/sbroad-core/src/ir/transformation/redistribution.rs
index e5ae28650a..8eb531af88 100644
--- a/sbroad-core/src/ir/transformation/redistribution.rs
+++ b/sbroad-core/src/ir/transformation/redistribution.rs
@@ -290,7 +290,7 @@ impl Plan {
) -> Result<MotionPolicy, SbroadError> {
let outer_dist = self.get_distribution(outer_id)?;
let inner_dist = self.get_distribution(inner_id)?;
- if Bool::Eq == *op || Bool::In == *op || Bool::NotEq == *op || Bool::NotIn == *op {
+ if Bool::Eq == *op || Bool::In == *op {
if let Distribution::Segment {
keys: ref keys_outer,
} = outer_dist
@@ -846,10 +846,15 @@ impl Plan {
}
(Expression::Row { .. }, Expression::Row { .. }) => {
match bool_op.op {
- Bool::Eq | Bool::In | Bool::NotEq | Bool::NotIn => {
+ Bool::Eq | Bool::In => {
self.join_policy_for_eq(rel_id, bool_op.left, bool_op.right)?
}
- Bool::Gt | Bool::GtEq | Bool::Lt | Bool::LtEq => MotionPolicy::Full,
+ Bool::Gt
+ | Bool::GtEq
+ | Bool::Lt
+ | Bool::LtEq
+ | Bool::NotEq
+ | Bool::NotIn => MotionPolicy::Full,
Bool::And | Bool::Or => {
// "a and 1" or "a or 1" expressions make no sense.
return Err(SbroadError::Unsupported(
diff --git a/sbroad-core/src/ir/transformation/redistribution/tests/not_in.rs b/sbroad-core/src/ir/transformation/redistribution/tests/not_in.rs
index f947356262..5a2302e682 100644
--- a/sbroad-core/src/ir/transformation/redistribution/tests/not_in.rs
+++ b/sbroad-core/src/ir/transformation/redistribution/tests/not_in.rs
@@ -2,7 +2,7 @@ use crate::ir::operator::Relational;
use crate::ir::transformation::helpers::sql_to_ir;
use crate::ir::transformation::redistribution::tests::get_motion_id;
use crate::ir::transformation::redistribution::{Key, MotionPolicy};
-use crate::ir::Slices;
+use crate::ir::{Slice, Slices};
use pretty_assertions::assert_eq;
#[test]
@@ -28,7 +28,7 @@ fn not_in2() {
let mut plan = sql_to_ir(query, vec![]);
plan.add_motions().unwrap();
- assert_eq!(Slices::empty(), plan.slices);
+ assert_eq!(Slices::from(vec![Slice { slice: vec![64] }]), plan.slices);
}
#[test]
@@ -41,15 +41,7 @@ fn not_in3() {
let motion_id = *get_motion_id(&plan, 0, 0).unwrap();
let motion = plan.get_relation_node(motion_id).unwrap();
if let Relational::Motion { policy, .. } = motion {
- assert_eq!(
- *policy,
- MotionPolicy::Segment(
- (Key {
- positions: vec![0, 1]
- })
- .into()
- )
- );
+ assert_eq!(*policy, MotionPolicy::Full);
} else {
panic!("Expected a motion node");
}
@@ -65,15 +57,7 @@ fn not_in4() {
let motion_id = *get_motion_id(&plan, 0, 0).unwrap();
let motion = plan.get_relation_node(motion_id).unwrap();
if let Relational::Motion { policy, .. } = motion {
- assert_eq!(
- *policy,
- MotionPolicy::Segment(
- (Key {
- positions: vec![1, 0]
- })
- .into()
- )
- );
+ assert_eq!(*policy, MotionPolicy::Full);
} else {
panic!("Expected a motion node");
}
@@ -89,15 +73,7 @@ fn not_in5() {
let motion_id = *get_motion_id(&plan, 0, 0).unwrap();
let motion = plan.get_relation_node(motion_id).unwrap();
if let Relational::Motion { policy, .. } = motion {
- assert_eq!(
- *policy,
- MotionPolicy::Segment(
- (Key {
- positions: vec![0, 1]
- })
- .into()
- )
- );
+ assert_eq!(*policy, MotionPolicy::Full);
} else {
panic!("Expected a motion node");
}
--
GitLab