From 8957cf712952d6cb5ad2ac6af78475b940efb4fd Mon Sep 17 00:00:00 2001
From: Denis Smirnov <sd@picodata.io>
Date: Tue, 9 Aug 2022 16:26:39 +0700
Subject: [PATCH] feat: implement between operator
---
doc/sql/feature_taxonomy.md | 2 +-
src/executor/tests.rs | 3 +
src/executor/tests/between.rs | 138 ++++++++++++++++++
src/frontend/sql.rs | 73 +++++----
src/frontend/sql/ast.rs | 2 +
src/frontend/sql/ir/tests.rs | 17 +++
src/frontend/sql/query.pest | 12 +-
src/ir/transformation/redistribution.rs | 2 +-
src/ir/transformation/redistribution/tests.rs | 5 +-
.../redistribution/tests/between.rs | 73 +++++++++
test_app/test/integration/api_test.lua | 39 +++++
11 files changed, 332 insertions(+), 34 deletions(-)
create mode 100644 src/executor/tests/between.rs
create mode 100644 src/ir/transformation/redistribution/tests/between.rs
diff --git a/doc/sql/feature_taxonomy.md b/doc/sql/feature_taxonomy.md
index 7adc449502..a27747b9be 100644
--- a/doc/sql/feature_taxonomy.md
+++ b/doc/sql/feature_taxonomy.md
@@ -113,7 +113,7 @@
1. Subclause 8.2, “comparison predicateâ€: For supported data types, without support for table subquery: **yes**
### E061-02. BETWEEN predicate.
-1. Subclause 8.3, “between predicateâ€: **no**
+1. Subclause 8.3, “between predicateâ€: **yes**
### E061-03. IN predicate with list of values.
1. Subclause 8.4, “in predicateâ€: Without support for table subquery: **yes**
diff --git a/src/executor/tests.rs b/src/executor/tests.rs
index d4e8a8364c..7d18caa9ea 100644
--- a/src/executor/tests.rs
+++ b/src/executor/tests.rs
@@ -1368,3 +1368,6 @@ fn get_motion_policy(plan: &Plan, motion_id: usize) -> &MotionPolicy {
panic!("Expected a motion node");
}
}
+
+#[cfg(test)]
+mod between;
diff --git a/src/executor/tests/between.rs b/src/executor/tests/between.rs
new file mode 100644
index 0000000000..fcc3957a2c
--- /dev/null
+++ b/src/executor/tests/between.rs
@@ -0,0 +1,138 @@
+use pretty_assertions::assert_eq;
+
+use crate::executor::engine::cartridge::backend::sql::ir::PatternWithParams;
+use crate::executor::engine::mock::RouterRuntimeMock;
+use crate::executor::result::ProducerResult;
+use crate::executor::vtable::VirtualTable;
+use crate::ir::relation::{Column, ColumnRole, Type};
+use crate::ir::transformation::redistribution::tests::get_motion_id;
+use crate::ir::transformation::redistribution::MotionPolicy;
+use crate::ir::value::Value;
+
+use super::*;
+
+#[test]
+fn between1_test() {
+ let sql = r#"
+ SELECT "identification_number" FROM "hash_testing" AS "t"
+ WHERE "identification_number" BETWEEN 1 AND (
+ SELECT "identification_number" as "id" FROM "hash_testing_hist"
+ WHERE "identification_number" = 2
+ )
+ "#;
+
+ // Initialize the query.
+ let coordinator = RouterRuntimeMock::new();
+ let mut query = Query::new(&coordinator, sql, &[]).unwrap();
+ let plan = query.exec_plan.get_ir_plan();
+
+ // Validate the motion type.
+ let motion_id = *get_motion_id(&plan, 0, 0).unwrap();
+ assert_eq!(&MotionPolicy::Full, get_motion_policy(plan, motion_id));
+
+ // Mock a virtual table.
+ let mut virtual_table = VirtualTable::new();
+ virtual_table.add_column(Column {
+ name: "id".into(),
+ r#type: Type::Integer,
+ role: ColumnRole::User,
+ });
+ virtual_table.add_tuple(vec![Value::from(2_u64)]);
+ query
+ .coordinator
+ .add_virtual_table(motion_id, virtual_table);
+
+ // Execute the query.
+ let result = *query
+ .dispatch()
+ .unwrap()
+ .downcast::<ProducerResult>()
+ .unwrap();
+
+ // Validate the result.
+ let mut expected = ProducerResult::new();
+ expected.rows.extend(vec![vec![
+ Value::String(format!("Execute query on all buckets")),
+ Value::String(String::from(PatternWithParams::new(
+ format!(
+ "{} {} {}",
+ r#"SELECT "t"."identification_number" FROM "hash_testing" as "t""#,
+ r#"WHERE ("t"."identification_number") <= (SELECT COLUMN_1 as "id" FROM (VALUES (?)))"#,
+ r#"and ("t"."identification_number") >= (?)"#,
+ ),
+ vec![
+ Value::from(2_u64),
+ Value::from(1_u64),
+ ],
+ ))),
+ ]]);
+ assert_eq!(expected, result);
+}
+
+#[test]
+fn between2_test() {
+ let sql = r#"
+ SELECT "identification_number" FROM "hash_testing" AS "t"
+ WHERE (
+ SELECT "identification_number" as "id" FROM "hash_testing_hist"
+ WHERE "identification_number" = 2
+ ) BETWEEN 1 and 3
+ "#;
+
+ // Initialize the query.
+ let coordinator = RouterRuntimeMock::new();
+ let mut query = Query::new(&coordinator, sql, &[]).unwrap();
+ let plan = query.exec_plan.get_ir_plan();
+
+ // Validate the motion type.
+ let motion1_id = *get_motion_id(plan, 0, 0).unwrap();
+ assert_eq!(&MotionPolicy::Full, get_motion_policy(plan, motion1_id));
+ let motion2_id = *get_motion_id(plan, 0, 1).unwrap();
+ assert_eq!(&MotionPolicy::Full, get_motion_policy(plan, motion2_id));
+ assert_eq!(true, get_motion_id(plan, 0, 2).is_none());
+
+ // Mock a virtual table.
+ let mut virtual_table = VirtualTable::new();
+ virtual_table.add_column(Column {
+ name: "id".into(),
+ r#type: Type::Integer,
+ role: ColumnRole::User,
+ });
+ virtual_table.add_tuple(vec![Value::from(2_u64)]);
+
+ // Bind the virtual table to both motions.
+ query
+ .coordinator
+ .add_virtual_table(motion1_id, virtual_table.clone());
+ query
+ .coordinator
+ .add_virtual_table(motion2_id, virtual_table);
+
+ // Execute the query.
+ let result = *query
+ .dispatch()
+ .unwrap()
+ .downcast::<ProducerResult>()
+ .unwrap();
+
+ // Validate the result.
+ let mut expected = ProducerResult::new();
+ expected.rows.extend(vec![vec![
+ Value::String(format!("Execute query on all buckets")),
+ Value::String(String::from(PatternWithParams::new(
+ format!(
+ "{} {} {}",
+ r#"SELECT "t"."identification_number" FROM "hash_testing" as "t""#,
+ r#"WHERE (SELECT COLUMN_1 as "id" FROM (VALUES (?))) <= (?)"#,
+ r#"and (SELECT COLUMN_2 as "id" FROM (VALUES (?))) >= (?)"#,
+ ),
+ vec![
+ Value::from(2_u64),
+ Value::from(3_u64),
+ Value::from(2_u64),
+ Value::from(1_u64),
+ ],
+ ))),
+ ]]);
+ assert_eq!(expected, result);
+}
diff --git a/src/frontend/sql.rs b/src/frontend/sql.rs
index aa1174ea59..8d8c1c049b 100644
--- a/src/frontend/sql.rs
+++ b/src/frontend/sql.rs
@@ -461,55 +461,57 @@ impl Ast for AbstractSyntaxTree {
| Type::Lt
| Type::LtEq
| Type::NotEq => {
- let mut to_row = |plan_id| -> Result<usize, QueryPlannerError> {
- if let Node::Expression(
- Expression::Reference { .. } | Expression::Constant { .. },
- ) = plan.get_node(plan_id)?
- {
- let row_id = plan.nodes.add_row(vec![plan_id], None);
- rows.insert(row_id);
- Ok(row_id)
- } else {
- Ok(plan_id)
- }
- };
let ast_left_id = node.children.get(0).ok_or_else(|| {
QueryPlannerError::CustomError(
"Left node id is not found among comparison children.".into(),
)
})?;
- let plan_left_id = to_row(map.get(*ast_left_id)?)?;
+ let plan_left_id = plan.as_row(map.get(*ast_left_id)?, &mut rows)?;
let ast_right_id = node.children.get(1).ok_or_else(|| {
QueryPlannerError::CustomError(
"Right node id is not found among comparison children.".into(),
)
})?;
- let plan_right_id = to_row(map.get(*ast_right_id)?)?;
+ let plan_right_id = plan.as_row(map.get(*ast_right_id)?, &mut rows)?;
let op = Bool::from_node_type(&node.rule)?;
let cond_id = plan.add_cond(plan_left_id, op, plan_right_id)?;
map.add(*id, cond_id);
}
Type::IsNull => {
- let mut to_row = |plan_id| -> Result<usize, QueryPlannerError> {
- if let Node::Expression(
- Expression::Reference { .. } | Expression::Constant { .. },
- ) = plan.get_node(plan_id)?
- {
- let row_id = plan.nodes.add_row(vec![plan_id], None);
- rows.insert(row_id);
- Ok(row_id)
- } else {
- Ok(plan_id)
- }
- };
let ast_child_id = node.children.get(0).ok_or_else(|| {
QueryPlannerError::CustomError(format!("{:?} has no children.", &node.rule))
})?;
- let plan_child_id = to_row(map.get(*ast_child_id)?)?;
+ let plan_child_id = plan.as_row(map.get(*ast_child_id)?, &mut rows)?;
let op = Unary::from_node_type(&node.rule)?;
let unary_id = plan.add_unary(op, plan_child_id)?;
map.add(*id, unary_id);
}
+ Type::Between => {
+ // left BETWEEN center AND right
+ let ast_left_id = node.children.get(0).ok_or_else(|| {
+ QueryPlannerError::CustomError(
+ "Left node id is not found among between children.".into(),
+ )
+ })?;
+ let plan_left_id = plan.as_row(map.get(*ast_left_id)?, &mut rows)?;
+ let ast_center_id = node.children.get(1).ok_or_else(|| {
+ QueryPlannerError::CustomError(
+ "Center node id is not found among between children.".into(),
+ )
+ })?;
+ let plan_center_id = plan.as_row(map.get(*ast_center_id)?, &mut rows)?;
+ let ast_right_id = node.children.get(2).ok_or_else(|| {
+ QueryPlannerError::CustomError(
+ "Right node id is not found among between children.".into(),
+ )
+ })?;
+ let plan_right_id = plan.as_row(map.get(*ast_right_id)?, &mut rows)?;
+
+ let greater_eq_id = plan.add_cond(plan_left_id, Bool::GtEq, plan_center_id)?;
+ let less_eq_id = plan.add_cond(plan_left_id, Bool::LtEq, plan_right_id)?;
+ let and_id = plan.add_cond(greater_eq_id, Bool::And, less_eq_id)?;
+ map.add(*id, and_id);
+ }
Type::Condition => {
let ast_child_id = node.children.get(0).ok_or_else(|| {
QueryPlannerError::CustomError("Condition has no children.".into())
@@ -870,6 +872,23 @@ impl Plan {
Ok(())
}
+
+ /// Wrap references and constants in the plan into rows.
+ fn as_row(
+ &mut self,
+ expr_id: usize,
+ rows: &mut HashSet<usize>,
+ ) -> Result<usize, QueryPlannerError> {
+ if let Node::Expression(Expression::Reference { .. } | Expression::Constant { .. }) =
+ self.get_node(expr_id)?
+ {
+ let row_id = self.nodes.add_row(vec![expr_id], None);
+ rows.insert(row_id);
+ Ok(row_id)
+ } else {
+ Ok(expr_id)
+ }
+ }
}
pub mod ast;
diff --git a/src/frontend/sql/ast.rs b/src/frontend/sql/ast.rs
index ace6fc9e2c..5f8a1729bc 100644
--- a/src/frontend/sql/ast.rs
+++ b/src/frontend/sql/ast.rs
@@ -28,6 +28,7 @@ pub enum Type {
AliasName,
And,
Asterisk,
+ Between,
Column,
ColumnName,
Condition,
@@ -80,6 +81,7 @@ impl Type {
Rule::AliasName => Ok(Type::AliasName),
Rule::And => Ok(Type::And),
Rule::Asterisk => Ok(Type::Asterisk),
+ Rule::Between => Ok(Type::Between),
Rule::Column => Ok(Type::Column),
Rule::ColumnName => Ok(Type::ColumnName),
Rule::Condition => Ok(Type::Condition),
diff --git a/src/frontend/sql/ir/tests.rs b/src/frontend/sql/ir/tests.rs
index dc7b8a1b21..72f6b06322 100644
--- a/src/frontend/sql/ir/tests.rs
+++ b/src/frontend/sql/ir/tests.rs
@@ -387,6 +387,23 @@ fn front_sql17() {
assert_eq!(sql_to_sql(input, &[], &no_transform), expected);
}
+#[test]
+fn front_sql18() {
+ let input = r#"SELECT "product_code" FROM "hash_testing"
+ WHERE "product_code" BETWEEN 1 AND 2"#;
+ let expected = PatternWithParams::new(
+ format!(
+ "{} {} {}",
+ r#"SELECT "hash_testing"."product_code""#,
+ r#"FROM "hash_testing" WHERE ("hash_testing"."product_code") >= (?)"#,
+ r#"and ("hash_testing"."product_code") <= (?)"#,
+ ),
+ vec![Value::from(1_u64), Value::from(2_u64)],
+ );
+
+ assert_eq!(sql_to_sql(input, &[], &no_transform), expected);
+}
+
#[test]
fn front_params1() {
let pattern = r#"SELECT "id", "FIRST_NAME" FROM "test_space"
diff --git a/src/frontend/sql/query.pest b/src/frontend/sql/query.pest
index 440ffea3e7..f8bf690c45 100644
--- a/src/frontend/sql/query.pest
+++ b/src/frontend/sql/query.pest
@@ -28,9 +28,11 @@ Query = _{ Except | UnionAll | Select | Values | Insert }
Values = { ^"values" ~ ValuesRow ~ ("," ~ ValuesRow)*? }
ValuesRow = { Row }
-Expr = _{ Or | And | Unary | Cmp | Primary | Parentheses }
+Expr = _{ Or | And | Unary | Between | Cmp | Primary | Parentheses }
Parentheses = _{ "(" ~ Expr ~ ")" }
Primary = _{ SubQuery | Value | Reference }
+ Unary = _{ IsNull }
+ IsNull = { Primary ~ ^"is" ~ ^"null" }
Cmp = _{ Eq | In | Gt | GtEq | Lt | LtEq | NotEq }
Eq = { EqLeft ~ "=" ~ EqRight }
EqLeft = _{ Primary }
@@ -53,10 +55,12 @@ Expr = _{ Or | And | Unary | Cmp | Primary | Parentheses }
NotEq = { NotEqLeft ~ ("<>" | "!=") ~ NotEqRight }
NotEqLeft = _{ LtEqRight }
NotEqRight = _{ NotEq | NotEqLeft }
- Unary = _{ IsNull }
- IsNull = { Primary ~ ^"is" ~ ^"null" }
+ Between = { BetweenLeft ~ ^"between" ~ BetweenCenter ~ ^"and" ~ BetweenRight }
+ BetweenLeft = _{ Primary }
+ BetweenCenter = _{ Primary }
+ BetweenRight = _{ Primary }
And = { AndLeft ~ ^"and" ~ AndRight }
- AndLeft = _{ Unary | Cmp | Primary | Parentheses }
+ AndLeft = _{ Unary | Between | Cmp | Primary | Parentheses }
AndRight = _{ And | AndLeft }
Or = { OrLeft ~ ^"or" ~ OrRight }
OrLeft = _{ AndRight }
diff --git a/src/ir/transformation/redistribution.rs b/src/ir/transformation/redistribution.rs
index 55bdbf2f52..603990898c 100644
--- a/src/ir/transformation/redistribution.rs
+++ b/src/ir/transformation/redistribution.rs
@@ -1014,4 +1014,4 @@ impl Plan {
}
#[cfg(test)]
-mod tests;
+pub mod tests;
diff --git a/src/ir/transformation/redistribution/tests.rs b/src/ir/transformation/redistribution/tests.rs
index c6dc0f3cb3..fb311fece8 100644
--- a/src/ir/transformation/redistribution/tests.rs
+++ b/src/ir/transformation/redistribution/tests.rs
@@ -872,10 +872,13 @@ fn redistribution6() {
}
/// Helper function to extract a motion id from a plan.
-fn get_motion_id(plan: &Plan, slice_id: usize, motion_idx: usize) -> Option<&usize> {
+pub fn get_motion_id(plan: &Plan, slice_id: usize, motion_idx: usize) -> Option<&usize> {
let slice = plan.slices.as_ref().unwrap().get(slice_id).unwrap();
slice.get(motion_idx)
}
+#[cfg(test)]
+mod between;
+
#[cfg(test)]
mod except;
diff --git a/src/ir/transformation/redistribution/tests/between.rs b/src/ir/transformation/redistribution/tests/between.rs
new file mode 100644
index 0000000000..f6f3f6dd7e
--- /dev/null
+++ b/src/ir/transformation/redistribution/tests/between.rs
@@ -0,0 +1,73 @@
+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::*;
+use pretty_assertions::assert_eq;
+
+#[test]
+fn between1() {
+ let query = r#"SELECT "identification_number" FROM "hash_testing" AS "t"
+ WHERE (SELECT "identification_number" FROM "hash_testing_hist" AS "h"
+ WHERE "identification_number" = 2) BETWEEN 1 AND 2"#;
+
+ let mut plan = sql_to_ir(query, &[]);
+ plan.add_motions().unwrap();
+ let motion_gt_eq_id = *get_motion_id(&plan, 0, 0).unwrap();
+ let motion = plan.get_relation_node(motion_gt_eq_id).unwrap();
+ if let Relational::Motion { policy, .. } = motion {
+ assert_eq!(*policy, MotionPolicy::Full);
+ } else {
+ panic!("Expected a motion node");
+ }
+ let motion_lt_eq_id = *get_motion_id(&plan, 0, 1).unwrap();
+ let motion = plan.get_relation_node(motion_lt_eq_id).unwrap();
+ if let Relational::Motion { policy, .. } = motion {
+ assert_eq!(*policy, MotionPolicy::Full);
+ } else {
+ panic!("Expected a motion node");
+ }
+ let no_other_motions = get_motion_id(&plan, 0, 2).is_none();
+ assert_eq!(no_other_motions, true);
+}
+
+#[test]
+fn between2() {
+ let query = r#"SELECT "identification_number" FROM "hash_testing" AS "t"
+ WHERE "identification_number" BETWEEN 1 AND 2"#;
+
+ let mut plan = sql_to_ir(query, &[]);
+ plan.add_motions().unwrap();
+ let expected: Option<Vec<Vec<usize>>> = None;
+ assert_eq!(expected, plan.slices);
+}
+
+#[test]
+fn between3() {
+ let query = r#"SELECT "identification_number" FROM "hash_testing" AS "t"
+ WHERE "identification_number" BETWEEN 1 AND (
+ SELECT "identification_number" FROM "hash_testing_hist"
+ WHERE "identification_number" = 3)"#;
+
+ let mut plan = sql_to_ir(query, &[]);
+ plan.add_motions().unwrap();
+ 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::Full);
+ } else {
+ panic!("Expected a motion node");
+ }
+ let no_other_motions = get_motion_id(&plan, 0, 2).is_none();
+ assert_eq!(no_other_motions, true);
+}
+
+#[test]
+fn between4() {
+ let query = r#"SELECT "identification_number" FROM "hash_testing" AS "t"
+ WHERE 1 BETWEEN "identification_number" AND 2"#;
+
+ let mut plan = sql_to_ir(query, &[]);
+ plan.add_motions().unwrap();
+ let expected: Option<Vec<Vec<usize>>> = None;
+ assert_eq!(expected, plan.slices);
+}
diff --git a/test_app/test/integration/api_test.lua b/test_app/test/integration/api_test.lua
index 1b118c1f34..c4e5d29598 100644
--- a/test_app/test/integration/api_test.lua
+++ b/test_app/test/integration/api_test.lua
@@ -680,3 +680,42 @@ g.test_is_null = function()
},
})
end
+
+g.test_between1 = function()
+ local api = cluster:server("api-1").net_box
+
+ local r, err = api:call("query", { [[
+ SELECT "id" FROM "space_simple_shard_key" WHERE
+ (SELECT "id" FROM "space_simple_shard_key_hist" WHERE "id" = 2) BETWEEN 1 AND 2
+ ]], {} })
+
+ t.assert_equals(err, nil)
+ t.assert_equals(r, {
+ metadata = {
+ {name = "id", type = "integer"},
+ },
+ rows = {
+ {1},
+ {10}
+ },
+ })
+end
+
+g.test_between2 = function()
+ local api = cluster:server("api-1").net_box
+
+ local r, err = api:call("query", { [[
+ SELECT "id" FROM "space_simple_shard_key" WHERE
+ "id" BETWEEN 1 AND 2
+ ]], {} })
+
+ t.assert_equals(err, nil)
+ t.assert_equals(r, {
+ metadata = {
+ {name = "id", type = "integer"},
+ },
+ rows = {
+ {1}
+ },
+ })
+end
--
GitLab