From 829955a538529e0383441c27ff24c59ea8f3f964 Mon Sep 17 00:00:00 2001 From: Artem Zuikov Date: Wed, 5 Aug 2020 12:31:59 +0300 Subject: [PATCH] Better errors in JOIN ON section (#13330) --- src/Interpreters/CollectJoinOnKeysVisitor.cpp | 57 ++++++++++--------- src/Interpreters/CollectJoinOnKeysVisitor.h | 5 +- .../01429_join_on_error_messages.reference | 0 .../01429_join_on_error_messages.sql | 11 ++++ 4 files changed, 43 insertions(+), 30 deletions(-) create mode 100644 tests/queries/0_stateless/01429_join_on_error_messages.reference create mode 100644 tests/queries/0_stateless/01429_join_on_error_messages.sql diff --git a/src/Interpreters/CollectJoinOnKeysVisitor.cpp b/src/Interpreters/CollectJoinOnKeysVisitor.cpp index 2e8f17edb81..e0fce4854d2 100644 --- a/src/Interpreters/CollectJoinOnKeysVisitor.cpp +++ b/src/Interpreters/CollectJoinOnKeysVisitor.cpp @@ -11,6 +11,8 @@ namespace ErrorCodes { extern const int INVALID_JOIN_ON_EXPRESSION; extern const int AMBIGUOUS_COLUMN_NAME; + extern const int SYNTAX_ERROR; + extern const int NOT_IMPLEMENTED; extern const int LOGICAL_ERROR; } @@ -54,47 +56,58 @@ void CollectJoinOnKeysMatcher::Data::asofToJoinKeys() addJoinKeys(asof_left_key, asof_right_key, {1, 2}); } - void CollectJoinOnKeysMatcher::visit(const ASTFunction & func, const ASTPtr & ast, Data & data) { if (func.name == "and") return; /// go into children - if (func.name == "equals") + if (func.name == "or") + throw Exception("JOIN ON does not support OR. Unexpected '" + queryToString(ast) + "'", ErrorCodes::NOT_IMPLEMENTED); + + ASOF::Inequality inequality = ASOF::getInequality(func.name); + if (func.name == "equals" || inequality != ASOF::Inequality::None) { if (func.arguments->children.size() != 2) - { - throwSyntaxException("Function 'equals' takes two arguments, got '" - + func.formatForErrorMessage() + "' instead."); - } + throw Exception("Function " + func.name + " takes two arguments, got '" + func.formatForErrorMessage() + "' instead", + ErrorCodes::SYNTAX_ERROR); + } + else + throw Exception("Expected equality or inequality, got '" + queryToString(ast) + "'", ErrorCodes::INVALID_JOIN_ON_EXPRESSION); + + if (func.name == "equals") + { ASTPtr left = func.arguments->children.at(0); ASTPtr right = func.arguments->children.at(1); auto table_numbers = getTableNumbers(ast, left, right, data); data.addJoinKeys(left, right, table_numbers); - return; } - - ASOF::Inequality inequality = ASOF::getInequality(func.name); - - if (data.is_asof && (inequality != ASOF::Inequality::None)) + else if (inequality != ASOF::Inequality::None) { + if (!data.is_asof) + throw Exception("JOIN ON inequalities are not supported. Unexpected '" + queryToString(ast) + "'", + ErrorCodes::NOT_IMPLEMENTED); + if (data.asof_left_key || data.asof_right_key) - throwSyntaxException("ASOF JOIN expects exactly one inequality in ON section, unexpected " + queryToString(ast) + "."); + throw Exception("ASOF JOIN expects exactly one inequality in ON section. Unexpected '" + queryToString(ast) + "'", + ErrorCodes::INVALID_JOIN_ON_EXPRESSION); ASTPtr left = func.arguments->children.at(0); ASTPtr right = func.arguments->children.at(1); auto table_numbers = getTableNumbers(ast, left, right, data); data.addAsofJoinKeys(left, right, table_numbers, inequality); - return; } - - throwSyntaxException("Expected equals expression, got " + queryToString(ast) + "."); } void CollectJoinOnKeysMatcher::getIdentifiers(const ASTPtr & ast, std::vector & out) { - if (const auto * ident = ast->as()) + if (const auto * func = ast->as()) + { + if (func->name == "arrayJoin") + throw Exception("Not allowed function in JOIN ON. Unexpected '" + queryToString(ast) + "'", + ErrorCodes::INVALID_JOIN_ON_EXPRESSION); + } + else if (const auto * ident = ast->as()) { if (IdentifierSemantic::getColumnName(*ident)) out.push_back(ident); @@ -122,8 +135,8 @@ std::pair CollectJoinOnKeysMatcher::getTableNumbers(const ASTPtr auto left_name = queryToString(*left_identifiers[0]); auto right_name = queryToString(*right_identifiers[0]); - throwSyntaxException("In expression " + queryToString(expr) + " columns " + left_name + " and " + right_name - + " are from the same table but from different arguments of equal function."); + throw Exception("In expression " + queryToString(expr) + " columns " + left_name + " and " + right_name + + " are from the same table but from different arguments of equal function", ErrorCodes::INVALID_JOIN_ON_EXPRESSION); } return std::make_pair(left_idents_table, right_idents_table); @@ -214,12 +227,4 @@ size_t CollectJoinOnKeysMatcher::getTableForIdentifiers(std::vectoras()) - if (func->name == "equals") - return false; + return func->name == "and"; return true; } @@ -61,8 +60,6 @@ private: static std::pair getTableNumbers(const ASTPtr & expr, const ASTPtr & left_ast, const ASTPtr & right_ast, Data & data); static const ASTIdentifier * unrollAliases(const ASTIdentifier * identifier, const Aliases & aliases); static size_t getTableForIdentifiers(std::vector & identifiers, const Data & data); - - [[noreturn]] static void throwSyntaxException(const String & msg); }; /// Parse JOIN ON expression and collect ASTs for joined columns. diff --git a/tests/queries/0_stateless/01429_join_on_error_messages.reference b/tests/queries/0_stateless/01429_join_on_error_messages.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/01429_join_on_error_messages.sql b/tests/queries/0_stateless/01429_join_on_error_messages.sql new file mode 100644 index 00000000000..f9e2647f2e3 --- /dev/null +++ b/tests/queries/0_stateless/01429_join_on_error_messages.sql @@ -0,0 +1,11 @@ +SELECT 1 FROM (select 1 a) A JOIN (select 1 b) B ON (arrayJoin([1]) = B.b); -- { serverError 403 } +SELECT 1 FROM (select 1 a) A JOIN (select 1 b) B ON (A.a = arrayJoin([1])); -- { serverError 403 } + +SELECT 1 FROM (select 1 a) A JOIN (select 1 b) B ON equals(a); -- { serverError 62 } +SELECT 1 FROM (select 1 a) A JOIN (select 1 b) B ON less(a); -- { serverError 62 } + +SELECT 1 FROM (select 1 a) A JOIN (select 1 b) B ON a = b OR a = b; -- { serverError 48 } +SELECT 1 FROM (select 1 a) A JOIN (select 1 b) B ON a = b AND a > b; -- { serverError 48 } +SELECT 1 FROM (select 1 a) A JOIN (select 1 b) B ON a = b AND a < b; -- { serverError 48 } +SELECT 1 FROM (select 1 a) A JOIN (select 1 b) B ON a = b AND a >= b; -- { serverError 48 } +SELECT 1 FROM (select 1 a) A JOIN (select 1 b) B ON a = b AND a <= b; -- { serverError 48 }