Better errors in JOIN ON section (#13330)

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)
-        {
+            throw Exception("Function " + func.name + " takes two arguments, got '" + func.formatForErrorMessage() + "' instead",
-            throwSyntaxException("Function 'equals' takes two arguments, got '"
+                            ErrorCodes::SYNTAX_ERROR);
-                + func.formatForErrorMessage() + "' instead.");
+    }
-        }
+    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;
     }
-
+    else if (inequality != ASOF::Inequality::None)
-    ASOF::Inequality inequality = ASOF::getInequality(func.name);
-
-    if (data.is_asof && (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<const ASTIdentifier *> & out)
 {
-    if (const auto * ident = ast->as<ASTIdentifier>())
+    if (const auto * func = ast->as<ASTFunction>())
+    {
+        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<ASTIdentifier>())
     {
         if (IdentifierSemantic::getColumnName(*ident))
             out.push_back(ident);
@@ -122,8 +135,8 @@ std::pair<size_t, size_t> 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
+        throw Exception("In expression " + queryToString(expr) + " columns " + left_name + " and " + right_name
-                                + " are from the same table but from different arguments of equal function.");
+            + " 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::vector<const ASTIde
     return table_number;
 }
 
-[[noreturn]] void CollectJoinOnKeysMatcher::throwSyntaxException(const String & msg)
-{
-    throw Exception("Invalid expression for JOIN ON. " + msg +
-        " Supported syntax: JOIN ON Expr([table.]column, ...) = Expr([table.]column, ...) "
-        "[AND Expr([table.]column, ...) = Expr([table.]column, ...) ...]",
-        ErrorCodes::INVALID_JOIN_ON_EXPRESSION);
-}
-
 }

src/Interpreters/CollectJoinOnKeysVisitor.h

@@ -49,8 +49,7 @@ public:
     static bool needChildVisit(const ASTPtr & node, const ASTPtr &)
     {
         if (auto * func = node->as<ASTFunction>())
-            if (func->name == "equals")
+            return func->name == "and";
-                return false;
         return true;
     }
 
@@ -61,8 +60,6 @@ private:
     static std::pair<size_t, size_t> 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<const ASTIdentifier *> & identifiers, const Data & data);
-
-    [[noreturn]] static void throwSyntaxException(const String & msg);
 };
 
 /// Parse JOIN ON expression and collect ASTs for joined columns.

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 }