From 2995b69f4a954910499248d217d387c2c97eaa5c Mon Sep 17 00:00:00 2001 From: vdimir Date: Wed, 18 May 2022 19:31:14 +0000 Subject: [PATCH] Swap order of converting_join_columns and before_join steps --- src/Interpreters/ExpressionAnalyzer.cpp | 11 ++++++----- src/Interpreters/ExpressionAnalyzer.h | 4 ++-- src/Interpreters/InterpreterSelectQuery.cpp | 18 +++++++++--------- .../02302_clash_const_aggegate_join.reference | 1 + .../02302_clash_const_aggegate_join.sql | 15 +++++++++++++++ 5 files changed, 33 insertions(+), 16 deletions(-) create mode 100644 tests/queries/0_stateless/02302_clash_const_aggegate_join.reference create mode 100644 tests/queries/0_stateless/02302_clash_const_aggegate_join.sql diff --git a/src/Interpreters/ExpressionAnalyzer.cpp b/src/Interpreters/ExpressionAnalyzer.cpp index 0b1154f6fd1..14bb8f8e8c2 100644 --- a/src/Interpreters/ExpressionAnalyzer.cpp +++ b/src/Interpreters/ExpressionAnalyzer.cpp @@ -976,16 +976,16 @@ ArrayJoinActionPtr SelectQueryExpressionAnalyzer::appendArrayJoin(ExpressionActi return array_join; } -bool SelectQueryExpressionAnalyzer::appendJoinLeftKeys(ExpressionActionsChain & chain, bool only_types) +void SelectQueryExpressionAnalyzer::appendJoinLeftKeys(ExpressionActionsChain & chain, bool only_types) { ExpressionActionsChain::Step & step = chain.lastStep(columns_after_array_join); getRootActions(analyzedJoin().leftKeysList(), only_types, step.actions()); - return true; } -JoinPtr SelectQueryExpressionAnalyzer::appendJoin(ExpressionActionsChain & chain, ActionsDAGPtr & converting_join_columns) +JoinPtr SelectQueryExpressionAnalyzer::appendJoin(ExpressionActionsChain & chain, ActionsDAGPtr & converting_join_columns, ActionsDAGPtr & before_join) { + const ColumnsWithTypeAndName & left_sample_columns = chain.getLastStep().getResultColumns(); JoinPtr table_join = makeTableJoin(*syntax->ast_join, left_sample_columns, converting_join_columns); @@ -995,6 +995,8 @@ JoinPtr SelectQueryExpressionAnalyzer::appendJoin(ExpressionActionsChain & chain chain.addStep(); } + before_join = chain.getLastActions(); + ExpressionActionsChain::Step & step = chain.lastStep(columns_after_array_join); chain.steps.push_back(std::make_unique( syntax->analyzed_join, table_join, step.getResultColumns())); @@ -1762,8 +1764,7 @@ ExpressionAnalysisResult::ExpressionAnalysisResult( if (query_analyzer.hasTableJoin()) { query_analyzer.appendJoinLeftKeys(chain, only_types || !first_stage); - before_join = chain.getLastActions(); - join = query_analyzer.appendJoin(chain, converting_join_columns); + join = query_analyzer.appendJoin(chain, converting_join_columns, before_join); chain.addStep(); } diff --git a/src/Interpreters/ExpressionAnalyzer.h b/src/Interpreters/ExpressionAnalyzer.h index 85efb3829d0..ef97fbf175a 100644 --- a/src/Interpreters/ExpressionAnalyzer.h +++ b/src/Interpreters/ExpressionAnalyzer.h @@ -377,8 +377,8 @@ private: /// Before aggregation: ArrayJoinActionPtr appendArrayJoin(ExpressionActionsChain & chain, ActionsDAGPtr & before_array_join, bool only_types); - bool appendJoinLeftKeys(ExpressionActionsChain & chain, bool only_types); - JoinPtr appendJoin(ExpressionActionsChain & chain, ActionsDAGPtr & converting_join_columns); + void appendJoinLeftKeys(ExpressionActionsChain & chain, bool only_types); + JoinPtr appendJoin(ExpressionActionsChain & chain, ActionsDAGPtr & converting_join_columns, ActionsDAGPtr & before_join); /// remove_filter is set in ExpressionActionsChain::finalize(); /// Columns in `additional_required_columns` will not be removed (they can be used for e.g. sampling or FINAL modifier). diff --git a/src/Interpreters/InterpreterSelectQuery.cpp b/src/Interpreters/InterpreterSelectQuery.cpp index 6bfadc66352..cfb64366e9b 100644 --- a/src/Interpreters/InterpreterSelectQuery.cpp +++ b/src/Interpreters/InterpreterSelectQuery.cpp @@ -1251,15 +1251,6 @@ void InterpreterSelectQuery::executeImpl(QueryPlan & query_plan, std::optional

( - query_plan.getCurrentDataStream(), - expressions.before_join); - before_join_step->setStepDescription("Before JOIN"); - query_plan.addStep(std::move(before_join_step)); - } - /// Optional step to convert key columns to common supertype. if (expressions.converting_join_columns) { @@ -1270,6 +1261,15 @@ void InterpreterSelectQuery::executeImpl(QueryPlan & query_plan, std::optional

( + query_plan.getCurrentDataStream(), + expressions.before_join); + before_join_step->setStepDescription("Before JOIN"); + query_plan.addStep(std::move(before_join_step)); + } + if (expressions.hasJoin()) { if (expressions.join->isFilled()) diff --git a/tests/queries/0_stateless/02302_clash_const_aggegate_join.reference b/tests/queries/0_stateless/02302_clash_const_aggegate_join.reference new file mode 100644 index 00000000000..d00491fd7e5 --- /dev/null +++ b/tests/queries/0_stateless/02302_clash_const_aggegate_join.reference @@ -0,0 +1 @@ +1 diff --git a/tests/queries/0_stateless/02302_clash_const_aggegate_join.sql b/tests/queries/0_stateless/02302_clash_const_aggegate_join.sql new file mode 100644 index 00000000000..979a19340d0 --- /dev/null +++ b/tests/queries/0_stateless/02302_clash_const_aggegate_join.sql @@ -0,0 +1,15 @@ +DROP TABLE IF EXISTS e; + +-- https://github.com/ClickHouse/ClickHouse/issues/36891 + +CREATE TABLE e ( a UInt64, t DateTime ) ENGINE = MergeTree PARTITION BY toDate(t) ORDER BY tuple(); + +INSERT INTO e SELECT 1, toDateTime('2020-02-01 12:00:01') + INTERVAL number MONTH FROM numbers(10); + +SELECT any('1') +FROM e JOIN ( SELECT 1 :: UInt32 AS key) AS da ON key = a +PREWHERE toString(a) = '1'; + +-- SELECT sumIf( 1, if( 1, toDateTime('2020-01-01 00:00:00', 'UTC'), toDateTime('1970-01-01 00:00:00', 'UTC')) > t ) +-- FROM e JOIN ( SELECT 1 joinKey) AS da ON joinKey = a +-- WHERE t >= toDateTime('2021-07-19T13:00:00', 'UTC') AND t <= toDateTime('2021-07-19T13:59:59', 'UTC');