Backport #50123 to 22.8: Fix crash with multiIf and constant condit…

…ion and nullable arguments
ClickHouse · May 24, 2023 · 95d46fc · 95d46fc
1 parent 9305607
commit 95d46fc
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 10 deletions.
diff --git a/src/Interpreters/InterpreterSelectQuery.cpp b/src/Interpreters/InterpreterSelectQuery.cpp
@@ -423,7 +423,6 @@ InterpreterSelectQuery::InterpreterSelectQuery(
     if (has_input || !joined_tables.resolveTables())
         joined_tables.makeFakeTable(storage, metadata_snapshot, source_header);
 
-
     if (context->getCurrentTransaction() && context->getSettingsRef().throw_on_unsupported_query_inside_transaction)
     {
         if (storage)

diff --git a/src/Interpreters/TreeOptimizer.cpp b/src/Interpreters/TreeOptimizer.cpp
@@ -779,8 +779,11 @@ void optimizeOrLikeChain(ASTPtr & query)
 
 }
 
-void TreeOptimizer::optimizeIf(ASTPtr & query, Aliases & aliases, bool if_chain_to_multiif)
+void TreeOptimizer::optimizeIf(ASTPtr & query, Aliases & aliases, bool if_chain_to_multiif, bool multiif_to_if)
 {
+    if (multiif_to_if)
+        optimizeMultiIfToIf(query);
+
     /// Optimize if with constant condition after constants was substituted instead of scalar subqueries.
     OptimizeIfWithConstantConditionVisitor(aliases).visit(query);
 
@@ -837,9 +840,6 @@ void TreeOptimizer::apply(ASTPtr & query, TreeRewriterResult & result,
     if (settings.optimize_normalize_count_variants)
         optimizeCountConstantAndSumOne(query);
 
-    if (settings.optimize_multiif_to_if)
-        optimizeMultiIfToIf(query);
-
     if (settings.optimize_rewrite_sum_if_to_count_if)
         optimizeSumIfFunctions(query);
 

diff --git a/src/Interpreters/TreeOptimizer.h b/src/Interpreters/TreeOptimizer.h
@@ -23,7 +23,8 @@ class TreeOptimizer
         const std::vector<TableWithColumnNamesAndTypes> & tables_with_columns,
         ContextPtr context);
 
-    static void optimizeIf(ASTPtr & query, Aliases & aliases, bool if_chain_to_multiif);
+
+    static void optimizeIf(ASTPtr & query, Aliases & aliases, bool if_chain_to_multiif, bool multiif_to_if);
     static void optimizeCountConstantAndSumOne(ASTPtr & query);
 };
 

diff --git a/src/Interpreters/TreeRewriter.cpp b/src/Interpreters/TreeRewriter.cpp
@@ -1289,10 +1289,15 @@ TreeRewriterResultPtr TreeRewriter::analyzeSelect(
     /// Push the predicate expression down to subqueries. The optimization should be applied to both initial and secondary queries.
     result.rewrite_subqueries = PredicateExpressionsOptimizer(getContext(), tables_with_columns, settings).optimize(*select_query);
 
-    TreeOptimizer::optimizeIf(query, result.aliases, settings.optimize_if_chain_to_multiif);
+     /// Only apply AST optimization for initial queries.
+    const bool ast_optimizations_allowed =
+        getContext()->getClientInfo().query_kind != ClientInfo::QueryKind::SECONDARY_QUERY
+        && !select_options.ignore_ast_optimizations;
 
-    /// Only apply AST optimization for initial queries.
-    if (getContext()->getClientInfo().query_kind != ClientInfo::QueryKind::SECONDARY_QUERY && !select_options.ignore_ast_optimizations)
+    bool optimize_multiif_to_if = ast_optimizations_allowed && settings.optimize_multiif_to_if;
+    TreeOptimizer::optimizeIf(query, result.aliases, settings.optimize_if_chain_to_multiif, optimize_multiif_to_if);
+
+    if (ast_optimizations_allowed)
         TreeOptimizer::apply(query, result, tables_with_columns, getContext());
 
     /// array_join_alias_to_name, array_join_result_to_source.
@@ -1382,7 +1387,7 @@ TreeRewriterResultPtr TreeRewriter::analyze(
     if (settings.legacy_column_name_of_tuple_literal)
         markTupleLiteralsAsLegacy(query);
 
-    TreeOptimizer::optimizeIf(query, result.aliases, settings.optimize_if_chain_to_multiif);
+    TreeOptimizer::optimizeIf(query, result.aliases, settings.optimize_if_chain_to_multiif, false);
 
     if (allow_aggregations)
     {

diff --git a/tests/queries/0_stateless/02751_multiif_to_if_crash.reference b/tests/queries/0_stateless/02751_multiif_to_if_crash.reference
@@ -0,0 +1 @@
+1
diff --git a/tests/queries/0_stateless/02751_multiif_to_if_crash.sql b/tests/queries/0_stateless/02751_multiif_to_if_crash.sql
@@ -0,0 +1 @@
+SELECT sum(A) FROM (SELECT multiIf(1, 1, NULL) as A);