Merge pull request #71823 from ClickHouse/backport/24.9/71432

Backport #71432 to 24.9: Fix crash with optimize_rewrite_array_exists_to_has

src/Analyzer/JoinNode.cpp

@@ -46,9 +46,15 @@ ASTPtr JoinNode::toASTTableJoin() const
         auto join_expression_ast = children[join_expression_child_index]->toAST();
 
         if (children[join_expression_child_index]->getNodeType() == QueryTreeNodeType::LIST)
+        {
             join_ast->using_expression_list = std::move(join_expression_ast);
+            join_ast->children.push_back(join_ast->using_expression_list);
+        }
         else
-            join_ast->on_expression = std::move(join_expression_ast);
+        {
+            join_ast->on_expression = join_expression_ast;
+            join_ast->children.push_back(join_ast->on_expression);
+        }
     }
 
     return join_ast;

src/Interpreters/ExecuteScalarSubqueriesVisitor.cpp

@@ -66,6 +66,18 @@ bool ExecuteScalarSubqueriesMatcher::needChildVisit(ASTPtr & node, const ASTPtr
             return false;
     }
 
+    if (auto * tables = node->as<ASTTablesInSelectQueryElement>())
+    {
+        /// Contrary to what's said in the code block above, ARRAY JOIN needs to resolve the subquery if possible
+        /// and assign an alias for 02367_optimize_trivial_count_with_array_join to pass. Otherwise it will fail in
+        /// ArrayJoinedColumnsVisitor (`No alias for non-trivial value in ARRAY JOIN: _a`)
+        /// This looks 100% as a incomplete code working on top of a bug, but this code has already been made obsolete
+        /// by the new analyzer, so it's an inconvenience we can live with until we deprecate it.
+        if (child == tables->array_join)
+            return true;
+        return false;
+    }
+
     return true;
 }
 

src/Interpreters/QueryNormalizer.cpp

@@ -161,7 +161,13 @@ void QueryNormalizer::visit(ASTTablesInSelectQueryElement & node, const ASTPtr &
     {
         auto & join = node.table_join->as<ASTTableJoin &>();
         if (join.on_expression)
+        {
+            ASTPtr original_on_expression = join.on_expression;
             visit(join.on_expression, data);
+            if (join.on_expression != original_on_expression)
+                join.children = { join.on_expression };
+        }
+
     }
 }
 

src/Interpreters/RewriteArrayExistsFunctionVisitor.cpp

@@ -6,6 +6,12 @@
 
 namespace DB
 {
+
+namespace ErrorCode
+{
+extern const int LOGICAL_ERROR;
+}
+
 void RewriteArrayExistsFunctionMatcher::visit(ASTPtr & ast, Data & data)
 {
     if (auto * func = ast->as<ASTFunction>())
@@ -20,21 +26,21 @@ void RewriteArrayExistsFunctionMatcher::visit(ASTPtr & ast, Data & data)
         if (join->using_expression_list)
         {
             auto * it = std::find(join->children.begin(), join->children.end(), join->using_expression_list);
+            if (it == join->children.end())
+                throw Exception(ErrorCodes::LOGICAL_ERROR, "Could not find join->using_expression_list in '{}'", join->formatForLogging());
 
             visit(join->using_expression_list, data);
-
+            *it = join->using_expression_list;
-            if (it && *it != join->using_expression_list)
-                *it = join->using_expression_list;
         }
 
         if (join->on_expression)
         {
             auto * it = std::find(join->children.begin(), join->children.end(), join->on_expression);
+            if (it == join->children.end())
+                throw Exception(ErrorCodes::LOGICAL_ERROR, "Could not find join->on_expression in '{}'", join->formatForLogging());
 
             visit(join->on_expression, data);
-
+            *it = join->on_expression;
-            if (it && *it != join->on_expression)
-                *it = join->on_expression;
         }
     }
 }

src/Storages/TimeSeries/PrometheusRemoteReadProtocol.cpp

@@ -231,6 +231,7 @@ namespace
             table_join->strictness = JoinStrictness::Semi;
 
             table_join->on_expression = makeASTFunction("equals", makeASTColumn(data_table_id, TimeSeriesColumnNames::ID), makeASTColumn(tags_table_id, TimeSeriesColumnNames::ID));
+            table_join->children.push_back(table_join->on_expression);
             table->table_join = table_join;
 
             auto table_exp = std::make_shared<ASTTableExpression>();

tests/docker_scripts/attach_gdb.lib

@@ -5,7 +5,8 @@ source /repo/tests/docker_scripts/utils.lib
 
 function attach_gdb_to_clickhouse()
 {
-    IS_ASAN=$(clickhouse-client --query "SELECT count() FROM system.build_options WHERE name = 'CXX_FLAGS' AND position('sanitize=address' IN value)")
+    # Use retries to avoid failures due to fault injections
+    IS_ASAN=$(run_with_retry 5 clickhouse-client --query "SELECT count() FROM system.build_options WHERE name = 'CXX_FLAGS' AND position('sanitize=address' IN value)")
     if [[ "$IS_ASAN" = "1" ]];
     then
         echo "ASAN build detected. Not using gdb since it disables LeakSanitizer detections"

tests/queries/0_stateless/03261_optimize_rewrite_array_exists_to_has_crash.sql

@@ -0,0 +1,10 @@
+-- https://github.com/ClickHouse/ClickHouse/issues/71382
+DROP TABLE IF EXISTS rewrite;
+CREATE TABLE rewrite (c0 Int) ENGINE = Memory();
+SELECT 1
+FROM rewrite
+INNER JOIN rewrite AS y ON (
+    SELECT 1
+)
+INNER JOIN rewrite AS z ON 1
+SETTINGS optimize_rewrite_array_exists_to_has=1;