Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash with multiIf and constant condition and nullable arguments #50123

Merged
merged 1 commit into from May 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion src/Interpreters/InterpreterSelectQuery.cpp
Expand Up @@ -440,7 +440,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)
Expand Down
8 changes: 4 additions & 4 deletions src/Interpreters/TreeOptimizer.cpp
Expand Up @@ -703,8 +703,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);

Expand Down Expand Up @@ -791,9 +794,6 @@ void TreeOptimizer::apply(ASTPtr & query, TreeRewriterResult & result,
if (settings.optimize_normalize_count_variants)
optimizeCountConstantAndSumOne(query, context);

if (settings.optimize_multiif_to_if)
optimizeMultiIfToIf(query);

if (settings.optimize_rewrite_sum_if_to_count_if)
optimizeSumIfFunctions(query);

Expand Down
2 changes: 1 addition & 1 deletion src/Interpreters/TreeOptimizer.h
Expand Up @@ -23,7 +23,7 @@ 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, ContextPtr context);
static void optimizeGroupByFunctionKeys(ASTSelectQuery * select_query);
};
Expand Down
13 changes: 8 additions & 5 deletions src/Interpreters/TreeRewriter.cpp
Expand Up @@ -1230,11 +1230,14 @@ 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;

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);

/// 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;
if (ast_optimizations_allowed)
TreeOptimizer::apply(query, result, tables_with_columns, getContext());

Expand Down Expand Up @@ -1341,7 +1344,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)
{
Expand Down
@@ -0,0 +1 @@
1
1 change: 1 addition & 0 deletions tests/queries/0_stateless/02751_multiif_to_if_crash.sql
@@ -0,0 +1 @@
SELECT sum(A) FROM (SELECT multiIf(1, 1, NULL) as A);