From 9dd0292d7707b62c946a2f29df73651972843c2c Mon Sep 17 00:00:00 2001 From: "NeZeD [Mac Pro]" Date: Sat, 2 Feb 2019 14:07:55 +0300 Subject: [PATCH 01/12] Implement 'NOT BETWEEN' --- dbms/src/Parsers/ExpressionListParsers.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dbms/src/Parsers/ExpressionListParsers.cpp b/dbms/src/Parsers/ExpressionListParsers.cpp index c732ce4f38e..df86d265c35 100644 --- a/dbms/src/Parsers/ExpressionListParsers.cpp +++ b/dbms/src/Parsers/ExpressionListParsers.cpp @@ -231,6 +231,7 @@ bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & exp /// For the expression (subject BETWEEN left AND right) /// create an AST the same as for (subject> = left AND subject <= right). + ParserKeyword s_not{"NOT"}; ParserKeyword s_between("BETWEEN"); ParserKeyword s_and("AND"); @@ -254,6 +255,10 @@ bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & exp if (!elem_parser.parse(pos, right, expected)) return false; + bool is_not = false; + if (s_not.ignore(pos, expected)) + is_not = true; + /// AND function auto f_and = std::make_shared(); auto args_and = std::make_shared(); @@ -272,11 +277,11 @@ bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & exp args_le->children.emplace_back(subject); args_le->children.emplace_back(right); - f_ge->name = "greaterOrEquals"; + f_ge->name = is_not ? "less" : "greaterOrEquals"; f_ge->arguments = args_ge; f_ge->children.emplace_back(f_ge->arguments); - f_le->name = "lessOrEquals"; + f_le->name = is_not ? "greater" : "lessOrEquals"; f_le->arguments = args_le; f_le->children.emplace_back(f_le->arguments); From 9265fb34a53758c6891d2dfce14763ac6c56c09b Mon Sep 17 00:00:00 2001 From: "NeZeD [Mac Pro]" Date: Sat, 2 Feb 2019 14:11:46 +0300 Subject: [PATCH 02/12] Implement teset for 'NOT BETWEEN' --- dbms/tests/queries/0_stateless/00834_not_between.reference | 1 + dbms/tests/queries/0_stateless/00834_not_between.sql | 1 + 2 files changed, 2 insertions(+) create mode 100644 dbms/tests/queries/0_stateless/00834_not_between.reference create mode 100644 dbms/tests/queries/0_stateless/00834_not_between.sql diff --git a/dbms/tests/queries/0_stateless/00834_not_between.reference b/dbms/tests/queries/0_stateless/00834_not_between.reference new file mode 100644 index 00000000000..d00491fd7e5 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00834_not_between.reference @@ -0,0 +1 @@ +1 diff --git a/dbms/tests/queries/0_stateless/00834_not_between.sql b/dbms/tests/queries/0_stateless/00834_not_between.sql new file mode 100644 index 00000000000..82e9f16ce2a --- /dev/null +++ b/dbms/tests/queries/0_stateless/00834_not_between.sql @@ -0,0 +1 @@ +SELECT 2 NOT BETWEEN 2 + 1 AND 4 - 1; From d605c9c1043e530029c06917f575ae14e4fdaefe Mon Sep 17 00:00:00 2001 From: "NeZeD [Mac Pro]" Date: Sat, 2 Feb 2019 14:23:25 +0300 Subject: [PATCH 03/12] Fix comparsion logic --- dbms/src/Parsers/ExpressionListParsers.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/dbms/src/Parsers/ExpressionListParsers.cpp b/dbms/src/Parsers/ExpressionListParsers.cpp index df86d265c35..940bb6adf8c 100644 --- a/dbms/src/Parsers/ExpressionListParsers.cpp +++ b/dbms/src/Parsers/ExpressionListParsers.cpp @@ -260,8 +260,8 @@ bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & exp is_not = true; /// AND function - auto f_and = std::make_shared(); - auto args_and = std::make_shared(); + auto f_combined_expression = std::make_shared(); + auto args_combined_expression = std::make_shared(); /// >= auto f_ge = std::make_shared(); @@ -285,14 +285,14 @@ bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & exp f_le->arguments = args_le; f_le->children.emplace_back(f_le->arguments); - args_and->children.emplace_back(f_ge); - args_and->children.emplace_back(f_le); + args_combined_expression->children.emplace_back(f_ge); + args_combined_expression->children.emplace_back(f_le); - f_and->name = "and"; - f_and->arguments = args_and; - f_and->children.emplace_back(f_and->arguments); + f_combined_expression->name = is_not ? "or" : "and"; + f_combined_expression->arguments = args_combined_expression; + f_combined_expression->children.emplace_back(f_combined_expression->arguments); - node = f_and; + node = args_combined_expression; } return true; From 764cf232a080f591a9b7e0cc5a8ffc536b42027c Mon Sep 17 00:00:00 2001 From: "NeZeD [Mac Pro]" Date: Sat, 2 Feb 2019 15:18:30 +0300 Subject: [PATCH 04/12] Consistient codestyle --- dbms/src/Parsers/ExpressionListParsers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Parsers/ExpressionListParsers.cpp b/dbms/src/Parsers/ExpressionListParsers.cpp index 940bb6adf8c..e302212432c 100644 --- a/dbms/src/Parsers/ExpressionListParsers.cpp +++ b/dbms/src/Parsers/ExpressionListParsers.cpp @@ -231,7 +231,7 @@ bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & exp /// For the expression (subject BETWEEN left AND right) /// create an AST the same as for (subject> = left AND subject <= right). - ParserKeyword s_not{"NOT"}; + ParserKeyword s_not("NOT"); ParserKeyword s_between("BETWEEN"); ParserKeyword s_and("AND"); From f53826779ab933fe3fc389fbbbc4e2d26427d42a Mon Sep 17 00:00:00 2001 From: "NeZeD [Mac Pro]" Date: Sat, 2 Feb 2019 15:20:18 +0300 Subject: [PATCH 05/12] Test `SELECT ... WHERE a NOT BETWEEN b AND c` --- dbms/tests/queries/0_stateless/00834_not_between.reference | 7 +++++++ dbms/tests/queries/0_stateless/00834_not_between.sql | 1 + 2 files changed, 8 insertions(+) diff --git a/dbms/tests/queries/0_stateless/00834_not_between.reference b/dbms/tests/queries/0_stateless/00834_not_between.reference index d00491fd7e5..76b56ea92ee 100644 --- a/dbms/tests/queries/0_stateless/00834_not_between.reference +++ b/dbms/tests/queries/0_stateless/00834_not_between.reference @@ -1 +1,8 @@ 1 +0 +1 +5 +6 +7 +8 +9 diff --git a/dbms/tests/queries/0_stateless/00834_not_between.sql b/dbms/tests/queries/0_stateless/00834_not_between.sql index 82e9f16ce2a..ea9b099e80e 100644 --- a/dbms/tests/queries/0_stateless/00834_not_between.sql +++ b/dbms/tests/queries/0_stateless/00834_not_between.sql @@ -1 +1,2 @@ SELECT 2 NOT BETWEEN 2 + 1 AND 4 - 1; +SELECT number FROM ( SELECT number FROM system.numbers LIMIT 10 ) WHERE number NOT BETWEEN 2 AND 4; From 2cabe8fcdb6eaf04eba6419c6b110a68d2c31748 Mon Sep 17 00:00:00 2001 From: "NeZeD [Mac Pro]" Date: Sat, 2 Feb 2019 18:29:37 +0300 Subject: [PATCH 06/12] Refactor code logic and make it readable --- dbms/src/Parsers/ExpressionListParsers.cpp | 56 ++++++++++++---------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/dbms/src/Parsers/ExpressionListParsers.cpp b/dbms/src/Parsers/ExpressionListParsers.cpp index e302212432c..652e3616aa5 100644 --- a/dbms/src/Parsers/ExpressionListParsers.cpp +++ b/dbms/src/Parsers/ExpressionListParsers.cpp @@ -228,7 +228,7 @@ bool ParserVariableArityOperatorList::parseImpl(Pos & pos, ASTPtr & node, Expect bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) { - /// For the expression (subject BETWEEN left AND right) + /// For the expression (subject [NOT] BETWEEN left AND right) /// create an AST the same as for (subject> = left AND subject <= right). ParserKeyword s_not("NOT"); @@ -255,40 +255,46 @@ bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & exp if (!elem_parser.parse(pos, right, expected)) return false; - bool is_not = false; - if (s_not.ignore(pos, expected)) - is_not = true; - - /// AND function auto f_combined_expression = std::make_shared(); auto args_combined_expression = std::make_shared(); - /// >= - auto f_ge = std::make_shared(); - auto args_ge = std::make_shared(); + /// [NOT] BETWEEN left AND right + auto f_left_expr = std::make_shared(); + auto args_left_expr = std::make_shared(); - /// <= - auto f_le = std::make_shared(); - auto args_le = std::make_shared(); + auto f_right_expr = std::make_shared(); + auto args_right_expr = std::make_shared(); - args_ge->children.emplace_back(subject); - args_ge->children.emplace_back(left); + args_left_expr->children.emplace_back(subject); + args_left_expr->children.emplace_back(left); - args_le->children.emplace_back(subject); - args_le->children.emplace_back(right); + args_right_expr->children.emplace_back(subject); + args_right_expr->children.emplace_back(right); - f_ge->name = is_not ? "less" : "greaterOrEquals"; - f_ge->arguments = args_ge; - f_ge->children.emplace_back(f_ge->arguments); + // NOT BETWEEN + if (s_not.ignore(pos, expected)) + { + f_left_expr->name = "less"; + f_right_expr->name = "greater"; + f_combined_expression->name = "or"; + } + // BETWEEN + else + { + f_left_expr->name = "greaterOrEquals"; + f_right_expr->name = "lessOrEquals"; + f_combined_expression->name = "and"; + } - f_le->name = is_not ? "greater" : "lessOrEquals"; - f_le->arguments = args_le; - f_le->children.emplace_back(f_le->arguments); + f_left_expr->arguments = args_left_expr; + f_left_expr->children.emplace_back(f_left_expr->arguments); - args_combined_expression->children.emplace_back(f_ge); - args_combined_expression->children.emplace_back(f_le); + f_right_expr->arguments = args_right_expr; + f_right_expr->children.emplace_back(f_right_expr->arguments); + + args_combined_expression->children.emplace_back(f_left_expr); + args_combined_expression->children.emplace_back(f_right_expr); - f_combined_expression->name = is_not ? "or" : "and"; f_combined_expression->arguments = args_combined_expression; f_combined_expression->children.emplace_back(f_combined_expression->arguments); From 8e564a616c012f25e835966e57bf0b51655dd6c7 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 10 Feb 2019 23:08:44 +0300 Subject: [PATCH 07/12] Fixed error #4228 --- dbms/src/Parsers/ExpressionListParsers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Parsers/ExpressionListParsers.cpp b/dbms/src/Parsers/ExpressionListParsers.cpp index 652e3616aa5..65f5f680865 100644 --- a/dbms/src/Parsers/ExpressionListParsers.cpp +++ b/dbms/src/Parsers/ExpressionListParsers.cpp @@ -298,7 +298,7 @@ bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & exp f_combined_expression->arguments = args_combined_expression; f_combined_expression->children.emplace_back(f_combined_expression->arguments); - node = args_combined_expression; + node = f_combined_expression; } return true; From 69b84380f2647b456f87dc0dd6742503122795f1 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 10 Feb 2019 23:17:53 +0300 Subject: [PATCH 08/12] Make it work #4228 --- dbms/src/Parsers/ExpressionListParsers.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dbms/src/Parsers/ExpressionListParsers.cpp b/dbms/src/Parsers/ExpressionListParsers.cpp index 65f5f680865..ed58d070b1c 100644 --- a/dbms/src/Parsers/ExpressionListParsers.cpp +++ b/dbms/src/Parsers/ExpressionListParsers.cpp @@ -242,8 +242,12 @@ bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & exp if (!elem_parser.parse(pos, subject, expected)) return false; + bool negative = s_not.ignore(pos, expected); + if (!s_between.ignore(pos, expected)) + { node = subject; + } else { if (!elem_parser.parse(pos, left, expected)) @@ -271,16 +275,16 @@ bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & exp args_right_expr->children.emplace_back(subject); args_right_expr->children.emplace_back(right); - // NOT BETWEEN - if (s_not.ignore(pos, expected)) + if (negative) { + /// NOT BETWEEN f_left_expr->name = "less"; f_right_expr->name = "greater"; f_combined_expression->name = "or"; } - // BETWEEN else { + /// BETWEEN f_left_expr->name = "greaterOrEquals"; f_right_expr->name = "lessOrEquals"; f_combined_expression->name = "and"; From 08c5c45f2b2b25ae74fb56e47be13c79b3a189e2 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 10 Feb 2019 23:18:11 +0300 Subject: [PATCH 09/12] Added a test for operator priority #4228 --- .../queries/0_stateless/00834_not_between.reference | 10 ++++++++++ dbms/tests/queries/0_stateless/00834_not_between.sql | 2 ++ 2 files changed, 12 insertions(+) diff --git a/dbms/tests/queries/0_stateless/00834_not_between.reference b/dbms/tests/queries/0_stateless/00834_not_between.reference index 76b56ea92ee..5a758d0e67c 100644 --- a/dbms/tests/queries/0_stateless/00834_not_between.reference +++ b/dbms/tests/queries/0_stateless/00834_not_between.reference @@ -6,3 +6,13 @@ 7 8 9 +0 0 +0 0 +0 0 +0 0 +1 1 +1 1 +1 1 +0 0 +0 0 +0 0 diff --git a/dbms/tests/queries/0_stateless/00834_not_between.sql b/dbms/tests/queries/0_stateless/00834_not_between.sql index ea9b099e80e..45709c7a1b7 100644 --- a/dbms/tests/queries/0_stateless/00834_not_between.sql +++ b/dbms/tests/queries/0_stateless/00834_not_between.sql @@ -1,2 +1,4 @@ SELECT 2 NOT BETWEEN 2 + 1 AND 4 - 1; SELECT number FROM ( SELECT number FROM system.numbers LIMIT 10 ) WHERE number NOT BETWEEN 2 AND 4; + +SELECT number BETWEEN 4 AND 6, NOT number NOT BETWEEN 4 AND 6 AND 1 FROM numbers(10); From 1f41aeaf7a47522adfe323edb029c724d832abc9 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 10 Feb 2019 23:18:47 +0300 Subject: [PATCH 10/12] Avoid some corner cases like "SELECT 1 NOT WITH CUBE" --- dbms/src/Parsers/ExpressionElementParsers.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dbms/src/Parsers/ExpressionElementParsers.cpp b/dbms/src/Parsers/ExpressionElementParsers.cpp index b3bfeae8bb7..e38689467ea 100644 --- a/dbms/src/Parsers/ExpressionElementParsers.cpp +++ b/dbms/src/Parsers/ExpressionElementParsers.cpp @@ -1130,6 +1130,9 @@ const char * ParserAlias::restricted_keywords[] = "FORMAT", "UNION", "INTO", + "NOT", + "BETWEEN", + "LIKE", nullptr }; From c0dd4debc59e345beaafbd69a47a0049f130a699 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 10 Feb 2019 23:21:22 +0300 Subject: [PATCH 11/12] Fixed error #4228 --- dbms/src/Parsers/ExpressionListParsers.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dbms/src/Parsers/ExpressionListParsers.cpp b/dbms/src/Parsers/ExpressionListParsers.cpp index ed58d070b1c..89aebb1d13e 100644 --- a/dbms/src/Parsers/ExpressionListParsers.cpp +++ b/dbms/src/Parsers/ExpressionListParsers.cpp @@ -246,6 +246,10 @@ bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & exp if (!s_between.ignore(pos, expected)) { + if (negative) + return false; /// Cannot parse NOT BETWEEN. + + /// No operator was parsed, just return element. node = subject; } else From f4f97867c35c72f7a2357f5d08171accafda36b5 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 10 Feb 2019 23:23:24 +0300 Subject: [PATCH 12/12] Fixed error #4228 --- dbms/src/Parsers/ExpressionListParsers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Parsers/ExpressionListParsers.cpp b/dbms/src/Parsers/ExpressionListParsers.cpp index 89aebb1d13e..953c5937dad 100644 --- a/dbms/src/Parsers/ExpressionListParsers.cpp +++ b/dbms/src/Parsers/ExpressionListParsers.cpp @@ -247,7 +247,7 @@ bool ParserBetweenExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & exp if (!s_between.ignore(pos, expected)) { if (negative) - return false; /// Cannot parse NOT BETWEEN. + --pos; /// No operator was parsed, just return element. node = subject;