Merge pull request #72305 from ClickHouse/backport/24.8/72179

Backport #72179 to 24.8: Fix bugs when using UDF in join on expression with the old analyzer

.github/PULL_REQUEST_TEMPLATE.md

@@ -12,7 +12,7 @@ tests/ci/cancel_and_rerun_workflow_lambda/app.py
 - Backward Incompatible Change
 - Build/Testing/Packaging Improvement
 - Documentation (changelog entry is not required)
-- Critical Bug Fix (crash, LOGICAL_ERROR, data loss, RBAC)
+- Critical Bug Fix (crash, data loss, RBAC) or LOGICAL_ERROR
 - Bug Fix (user-visible misbehavior in an official stable release)
 - CI Fix or Improvement (changelog entry is not required)
 - Not for changelog (changelog entry is not required)

src/Functions/UserDefined/UserDefinedSQLFunctionVisitor.cpp

@@ -26,14 +26,6 @@ void UserDefinedSQLFunctionVisitor::visit(ASTPtr & ast)
 {
     chassert(ast);
 
-    if (const auto * function = ast->template as<ASTFunction>())
-    {
-        std::unordered_set<std::string> udf_in_replace_process;
-        auto replace_result = tryToReplaceFunction(*function, udf_in_replace_process);
-        if (replace_result)
-            ast = replace_result;
-    }
-
     for (auto & child : ast->children)
     {
         if (!child)
@@ -48,6 +40,14 @@ void UserDefinedSQLFunctionVisitor::visit(ASTPtr & ast)
         if (new_ptr != old_ptr)
             ast->updatePointerToChild(old_ptr, new_ptr);
     }
+
+    if (const auto * function = ast->template as<ASTFunction>())
+    {
+        std::unordered_set<std::string> udf_in_replace_process;
+        auto replace_result = tryToReplaceFunction(*function, udf_in_replace_process);
+        if (replace_result)
+            ast = replace_result;
+    }
 }
 
 void UserDefinedSQLFunctionVisitor::visit(IAST * ast)

src/Interpreters/CrossToInnerJoinVisitor.cpp

@@ -70,7 +70,7 @@ struct JoinedElement
         join->strictness = JoinStrictness::All;
 
         join->on_expression = on_expression;
-        join->children.push_back(join->on_expression);
+        join->children = {join->on_expression};
         return true;
     }
 

src/Parsers/ASTColumnDeclaration.cpp

@@ -135,12 +135,25 @@ void ASTColumnDeclaration::formatImpl(const FormatSettings & format_settings, Fo
 
 void ASTColumnDeclaration::forEachPointerToChild(std::function<void(void **)> f)
 {
-    f(reinterpret_cast<void **>(&default_expression));
-    f(reinterpret_cast<void **>(&comment));
-    f(reinterpret_cast<void **>(&codec));
-    f(reinterpret_cast<void **>(&statistics_desc));
-    f(reinterpret_cast<void **>(&ttl));
-    f(reinterpret_cast<void **>(&collation));
-    f(reinterpret_cast<void **>(&settings));
+    auto visit_child = [&f](ASTPtr & member)
+    {
+        IAST * new_member_ptr = member.get();
+        f(reinterpret_cast<void **>(&new_member_ptr));
+        if (new_member_ptr != member.get())
+        {
+            if (new_member_ptr)
+                member = new_member_ptr->ptr();
+            else
+                member.reset();
+        }
+    };
+
+    visit_child(default_expression);
+    visit_child(comment);
+    visit_child(codec);
+    visit_child(statistics_desc);
+    visit_child(ttl);
+    visit_child(collation);
+    visit_child(settings);
 }
 }

src/Parsers/ASTTablesInSelectQuery.cpp

@@ -61,6 +61,29 @@ ASTPtr ASTTableJoin::clone() const
     return res;
 }
 
+void ASTTableJoin::forEachPointerToChild(std::function<void(void **)> f)
+{
+    IAST * new_using_expression_list = using_expression_list.get();
+    f(reinterpret_cast<void **>(&new_using_expression_list));
+    if (new_using_expression_list != using_expression_list.get())
+    {
+        if (new_using_expression_list)
+            using_expression_list = new_using_expression_list->ptr();
+        else
+            using_expression_list.reset();
+    }
+
+    IAST * new_on_expression = on_expression.get();
+    f(reinterpret_cast<void **>(&new_on_expression));
+    if (new_on_expression != on_expression.get())
+    {
+        if (new_on_expression)
+            on_expression = new_on_expression->ptr();
+        else
+            on_expression.reset();
+    }
+}
+
 void ASTArrayJoin::updateTreeHashImpl(SipHash & hash_state, bool ignore_aliases) const
 {
     hash_state.update(kind);

src/Parsers/ASTTablesInSelectQuery.h

@@ -80,6 +80,9 @@ struct ASTTableJoin : public IAST
     void formatImplAfterTable(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const;
     void formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override;
     void updateTreeHashImpl(SipHash & hash_state, bool ignore_aliases) const override;
+
+protected:
+    void forEachPointerToChild(std::function<void(void **)> f) override;
 };
 
 /// Specification of ARRAY JOIN.

tests/ci/run_check.py

@@ -56,7 +56,9 @@ LABEL_CATEGORIES = {
         "Bug Fix (user-visible misbehaviour in official stable or prestable release)",
         "Bug Fix (user-visible misbehavior in official stable or prestable release)",
     ],
-    "pr-critical-bugfix": ["Critical Bug Fix (crash, LOGICAL_ERROR, data loss, RBAC)"],
+    "pr-critical-bugfix": [
+        "Critical Bug Fix (crash, data loss, RBAC) or LOGICAL_ERROR"
+    ],
     "pr-build": [
         "Build/Testing/Packaging Improvement",
         "Build Improvement",

tests/queries/0_stateless/03141_fetches_errors_stress.sql

@@ -1,4 +1,4 @@
--- Tags: no-parallel
+-- Tags: no-fasttest, no-parallel
 -- Tag no-parallel -- due to failpoints
 
 create table data_r1 (key Int, value String) engine=ReplicatedMergeTree('/tables/{database}/data', '{table}') order by tuple();

tests/queries/0_stateless/03274_udf_in_join.reference

@@ -0,0 +1,7 @@
+SELECT 1
+FROM
+(
+    SELECT 1 AS c0
+) AS v0
+ALL INNER JOIN v0 AS vx ON c0 = vx.c0
+1

tests/queries/0_stateless/03274_udf_in_join.sh

@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+
+CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
+# shellcheck source=../shell_config.sh
+. "$CUR_DIR"/../shell_config.sh
+
+$CLICKHOUSE_CLIENT -q "
+  CREATE VIEW v0 AS SELECT 1 AS c0;
+  CREATE FUNCTION ${CLICKHOUSE_DATABASE}_second AS (x, y) -> y;
+  CREATE FUNCTION ${CLICKHOUSE_DATABASE}_equals AS (x, y) -> x = y;
+  SET optimize_rewrite_array_exists_to_has = 1;
+
+  EXPLAIN SYNTAX SELECT 1 FROM v0 JOIN v0 vx ON ${CLICKHOUSE_DATABASE}_second(v0.c0, vx.c0); -- { serverError INVALID_JOIN_ON_EXPRESSION }
+  EXPLAIN SYNTAX SELECT 1 FROM v0 JOIN v0 vx ON ${CLICKHOUSE_DATABASE}_equals(v0.c0, vx.c0);
+
+  SELECT 1 FROM v0 JOIN v0 vx ON ${CLICKHOUSE_DATABASE}_equals(v0.c0, vx.c0);
+
+  DROP view v0;
+  DROP FUNCTION ${CLICKHOUSE_DATABASE}_second;
+  DROP FUNCTION ${CLICKHOUSE_DATABASE}_equals;
+"