From cae2f023852f3f49c4a0f25e8e11bcb3fe6ed49a Mon Sep 17 00:00:00 2001 From: wudidapaopao <664920313@qq.com> Date: Fri, 14 Jun 2024 14:39:00 +0800 Subject: [PATCH 1/8] Fix unexpected projection name when query with CTE --- src/Analyzer/Resolve/QueryAnalyzer.cpp | 6 +++++- .../02378_analyzer_projection_names.reference | 12 ++++++++++++ .../0_stateless/02378_analyzer_projection_names.sql | 10 ++++++++++ .../0_stateless/03123_analyzer_dist_join_CTE.sql | 2 +- 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/Analyzer/Resolve/QueryAnalyzer.cpp b/src/Analyzer/Resolve/QueryAnalyzer.cpp index 5e5ecaaa93a..dc35462a4b0 100644 --- a/src/Analyzer/Resolve/QueryAnalyzer.cpp +++ b/src/Analyzer/Resolve/QueryAnalyzer.cpp @@ -1484,7 +1484,11 @@ void QueryAnalyzer::qualifyColumnNodesWithProjectionNames(const QueryTreeNodes & /// Build additional column qualification parts array std::vector additional_column_qualification_parts; - if (table_expression_node->hasAlias()) + auto * query_node = table_expression_node->as(); + auto * union_node = table_expression_node->as(); + bool is_cte = (query_node && query_node->isCTE()) || (union_node && union_node->isCTE()); + + if (!is_cte && table_expression_node->hasAlias()) additional_column_qualification_parts = {table_expression_node->getAlias()}; else if (auto * table_node = table_expression_node->as()) additional_column_qualification_parts = {table_node->getStorageID().getDatabaseName(), table_node->getStorageID().getTableName()}; diff --git a/tests/queries/0_stateless/02378_analyzer_projection_names.reference b/tests/queries/0_stateless/02378_analyzer_projection_names.reference index fd5bc7d4ae8..5860f1deb0d 100644 --- a/tests/queries/0_stateless/02378_analyzer_projection_names.reference +++ b/tests/queries/0_stateless/02378_analyzer_projection_names.reference @@ -443,6 +443,18 @@ SELECT '--'; -- DESCRIBE (WITH test_table_in_cte AS (SELECT id FROM test_table) SELECT id IN test_table_in_cte FROM test_table); in(id, test_table_in_cte) UInt8 +SELECT '--'; +-- +DESCRIBE (WITH test_table_in_cte_1 AS (SELECT 1 AS c1), test_table_in_cte_2 AS (SELECT 1 AS c1) SELECT * +FROM test_table_in_cte_1 INNER JOIN test_table_in_cte_2 as test_table_in_cte_2 ON test_table_in_cte_1.c1 = test_table_in_cte_2.c1); +c1 UInt8 +c1 UInt8 +SELECT '--'; +-- +DESCRIBE (WITH test_table_in_cte_1 AS (SELECT 1 AS c1), test_table_in_cte_2 AS (SELECT 1 AS c1 UNION ALL SELECT 1 AS c1) SELECT * +FROM test_table_in_cte_1 INNER JOIN test_table_in_cte_2 as test_table_in_cte_2 ON test_table_in_cte_1.c1 = test_table_in_cte_2.c1); +c1 UInt8 +c1 UInt8 SELECT 'Joins'; Joins DESCRIBE (SELECT * FROM test_table_join_1, test_table_join_2); diff --git a/tests/queries/0_stateless/02378_analyzer_projection_names.sql b/tests/queries/0_stateless/02378_analyzer_projection_names.sql index f5ac5f7476f..f41afe6a950 100644 --- a/tests/queries/0_stateless/02378_analyzer_projection_names.sql +++ b/tests/queries/0_stateless/02378_analyzer_projection_names.sql @@ -408,6 +408,16 @@ SELECT '--'; DESCRIBE (WITH test_table_in_cte AS (SELECT id FROM test_table) SELECT id IN test_table_in_cte FROM test_table); +SELECT '--'; + +DESCRIBE (WITH test_table_in_cte_1 AS (SELECT 1 AS c1), test_table_in_cte_2 AS (SELECT 1 AS c1) SELECT * +FROM test_table_in_cte_1 INNER JOIN test_table_in_cte_2 as test_table_in_cte_2 ON test_table_in_cte_1.c1 = test_table_in_cte_2.c1); + +SELECT '--'; + +DESCRIBE (WITH test_table_in_cte_1 AS (SELECT 1 AS c1), test_table_in_cte_2 AS (SELECT 1 AS c1 UNION ALL SELECT 1 AS c1) SELECT * +FROM test_table_in_cte_1 INNER JOIN test_table_in_cte_2 as test_table_in_cte_2 ON test_table_in_cte_1.c1 = test_table_in_cte_2.c1); + SELECT 'Joins'; DESCRIBE (SELECT * FROM test_table_join_1, test_table_join_2); diff --git a/tests/queries/0_stateless/03123_analyzer_dist_join_CTE.sql b/tests/queries/0_stateless/03123_analyzer_dist_join_CTE.sql index 4fb8e0b91c4..4275f405d13 100644 --- a/tests/queries/0_stateless/03123_analyzer_dist_join_CTE.sql +++ b/tests/queries/0_stateless/03123_analyzer_dist_join_CTE.sql @@ -20,7 +20,7 @@ WITH SELECT toInt64(number) AS a FROM numbers(10) ) -SELECT * +SELECT a.a, b, b.a, c.a FROM dist_t0 AS a LEFT JOIN b AS b ON a.a = b.a LEFT JOIN c AS c ON a.a = c.a From 409b24b8d8d185e7840bc082fd73afc1b13314d2 Mon Sep 17 00:00:00 2001 From: wudidapaopao <664920313@qq.com> Date: Tue, 18 Jun 2024 16:22:00 +0800 Subject: [PATCH 2/8] empty commit From 84fb836ecba44b0674f8e4bdbb1626e1aae02ba6 Mon Sep 17 00:00:00 2001 From: Yakov Olkhovskiy Date: Tue, 2 Jul 2024 23:27:02 +0000 Subject: [PATCH 3/8] proper qualification for CTE, fix tests --- src/Analyzer/Resolve/QueryAnalyzer.cpp | 10 +++++----- .../02378_analyzer_projection_names.reference | 4 ++-- .../0_stateless/03123_analyzer_dist_join_CTE.sql | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Analyzer/Resolve/QueryAnalyzer.cpp b/src/Analyzer/Resolve/QueryAnalyzer.cpp index dc35462a4b0..58a85debf37 100644 --- a/src/Analyzer/Resolve/QueryAnalyzer.cpp +++ b/src/Analyzer/Resolve/QueryAnalyzer.cpp @@ -1484,14 +1484,14 @@ void QueryAnalyzer::qualifyColumnNodesWithProjectionNames(const QueryTreeNodes & /// Build additional column qualification parts array std::vector additional_column_qualification_parts; - auto * query_node = table_expression_node->as(); - auto * union_node = table_expression_node->as(); - bool is_cte = (query_node && query_node->isCTE()) || (union_node && union_node->isCTE()); - - if (!is_cte && table_expression_node->hasAlias()) + if (table_expression_node->hasAlias()) additional_column_qualification_parts = {table_expression_node->getAlias()}; else if (auto * table_node = table_expression_node->as()) additional_column_qualification_parts = {table_node->getStorageID().getDatabaseName(), table_node->getStorageID().getTableName()}; + else if (auto * query_node = table_expression_node->as(); query_node && query_node->isCTE()) + additional_column_qualification_parts = {"", query_node->getCTEName()}; + else if (auto * union_node = table_expression_node->as(); union_node && union_node->isCTE()) + additional_column_qualification_parts = {"", union_node->getCTEName()}; size_t additional_column_qualification_parts_size = additional_column_qualification_parts.size(); const auto & table_expression_data = scope.getTableExpressionDataOrThrow(table_expression_node); diff --git a/tests/queries/0_stateless/02378_analyzer_projection_names.reference b/tests/queries/0_stateless/02378_analyzer_projection_names.reference index 5860f1deb0d..170f3e9e1ad 100644 --- a/tests/queries/0_stateless/02378_analyzer_projection_names.reference +++ b/tests/queries/0_stateless/02378_analyzer_projection_names.reference @@ -448,13 +448,13 @@ SELECT '--'; DESCRIBE (WITH test_table_in_cte_1 AS (SELECT 1 AS c1), test_table_in_cte_2 AS (SELECT 1 AS c1) SELECT * FROM test_table_in_cte_1 INNER JOIN test_table_in_cte_2 as test_table_in_cte_2 ON test_table_in_cte_1.c1 = test_table_in_cte_2.c1); c1 UInt8 -c1 UInt8 +test_table_in_cte_2.c1 UInt8 SELECT '--'; -- DESCRIBE (WITH test_table_in_cte_1 AS (SELECT 1 AS c1), test_table_in_cte_2 AS (SELECT 1 AS c1 UNION ALL SELECT 1 AS c1) SELECT * FROM test_table_in_cte_1 INNER JOIN test_table_in_cte_2 as test_table_in_cte_2 ON test_table_in_cte_1.c1 = test_table_in_cte_2.c1); c1 UInt8 -c1 UInt8 +test_table_in_cte_2.c1 UInt8 SELECT 'Joins'; Joins DESCRIBE (SELECT * FROM test_table_join_1, test_table_join_2); diff --git a/tests/queries/0_stateless/03123_analyzer_dist_join_CTE.sql b/tests/queries/0_stateless/03123_analyzer_dist_join_CTE.sql index 4275f405d13..4fb8e0b91c4 100644 --- a/tests/queries/0_stateless/03123_analyzer_dist_join_CTE.sql +++ b/tests/queries/0_stateless/03123_analyzer_dist_join_CTE.sql @@ -20,7 +20,7 @@ WITH SELECT toInt64(number) AS a FROM numbers(10) ) -SELECT a.a, b, b.a, c.a +SELECT * FROM dist_t0 AS a LEFT JOIN b AS b ON a.a = b.a LEFT JOIN c AS c ON a.a = c.a From 6128f53f63332d57e9e7f34e54dcd3b9f5c89df5 Mon Sep 17 00:00:00 2001 From: Yakov Olkhovskiy Date: Wed, 3 Jul 2024 02:56:47 +0000 Subject: [PATCH 4/8] fix --- src/Analyzer/Resolve/IdentifierResolver.cpp | 2 +- src/Analyzer/Resolve/QueryAnalyzer.cpp | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Analyzer/Resolve/IdentifierResolver.cpp b/src/Analyzer/Resolve/IdentifierResolver.cpp index 692a31b66ba..59cf82989e7 100644 --- a/src/Analyzer/Resolve/IdentifierResolver.cpp +++ b/src/Analyzer/Resolve/IdentifierResolver.cpp @@ -1129,7 +1129,7 @@ QueryTreeNodePtr IdentifierResolver::tryResolveIdentifierFromJoin(const Identifi resolved_identifier = left_resolved_identifier; } } - else if (scope.joins_count == 1 && scope.context->getSettingsRef().single_join_prefer_left_table) + else if (scope.joins_count && scope.context->getSettingsRef().single_join_prefer_left_table) { resolved_side = JoinTableSide::Left; resolved_identifier = left_resolved_identifier; diff --git a/src/Analyzer/Resolve/QueryAnalyzer.cpp b/src/Analyzer/Resolve/QueryAnalyzer.cpp index 58a85debf37..25ab3752f11 100644 --- a/src/Analyzer/Resolve/QueryAnalyzer.cpp +++ b/src/Analyzer/Resolve/QueryAnalyzer.cpp @@ -1489,9 +1489,9 @@ void QueryAnalyzer::qualifyColumnNodesWithProjectionNames(const QueryTreeNodes & else if (auto * table_node = table_expression_node->as()) additional_column_qualification_parts = {table_node->getStorageID().getDatabaseName(), table_node->getStorageID().getTableName()}; else if (auto * query_node = table_expression_node->as(); query_node && query_node->isCTE()) - additional_column_qualification_parts = {"", query_node->getCTEName()}; + additional_column_qualification_parts = {query_node->getCTEName()}; else if (auto * union_node = table_expression_node->as(); union_node && union_node->isCTE()) - additional_column_qualification_parts = {"", union_node->getCTEName()}; + additional_column_qualification_parts = {union_node->getCTEName()}; size_t additional_column_qualification_parts_size = additional_column_qualification_parts.size(); const auto & table_expression_data = scope.getTableExpressionDataOrThrow(table_expression_node); @@ -4455,10 +4455,11 @@ void QueryAnalyzer::initializeTableExpressionData(const QueryTreeNodePtr & table if (auto * scope_query_node = scope.scope_node->as()) { auto left_table_expression = extractLeftTableExpression(scope_query_node->getJoinTree()); + bool is_cte = (query_node && query_node->isCTE()) || (union_node && union_node->isCTE()); if (table_expression_node.get() == left_table_expression.get() && - scope.joins_count == 1 && - scope.context->getSettingsRef().single_join_prefer_left_table) - table_expression_data.should_qualify_columns = false; + scope.joins_count && + (scope.context->getSettingsRef().single_join_prefer_left_table || is_cte)) + table_expression_data.should_qualify_columns = false; } scope.table_expression_node_to_data.emplace(table_expression_node, std::move(table_expression_data)); From e11b48c7b5ef1bb25369f8a687b99b16b8ab198d Mon Sep 17 00:00:00 2001 From: Yakov Olkhovskiy Date: Wed, 3 Jul 2024 23:04:13 +0000 Subject: [PATCH 5/8] fix --- src/Analyzer/Resolve/QueryAnalyzer.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Analyzer/Resolve/QueryAnalyzer.cpp b/src/Analyzer/Resolve/QueryAnalyzer.cpp index 9e4d3fead6c..fc7e56c281f 100644 --- a/src/Analyzer/Resolve/QueryAnalyzer.cpp +++ b/src/Analyzer/Resolve/QueryAnalyzer.cpp @@ -4458,10 +4458,8 @@ void QueryAnalyzer::initializeTableExpressionData(const QueryTreeNodePtr & table if (auto * scope_query_node = scope.scope_node->as()) { auto left_table_expression = extractLeftTableExpression(scope_query_node->getJoinTree()); - bool is_cte = (query_node && query_node->isCTE()) || (union_node && union_node->isCTE()); if (table_expression_node.get() == left_table_expression.get() && - scope.joins_count && - (scope.context->getSettingsRef().single_join_prefer_left_table || is_cte)) + scope.joins_count && scope.context->getSettingsRef().single_join_prefer_left_table) table_expression_data.should_qualify_columns = false; } From 8fded210bc7320d144bdccfa05feabf0b07ce67e Mon Sep 17 00:00:00 2001 From: Yakov Olkhovskiy Date: Fri, 5 Jul 2024 14:49:19 +0000 Subject: [PATCH 6/8] revert to one join --- src/Analyzer/Resolve/IdentifierResolver.cpp | 2 +- src/Analyzer/Resolve/QueryAnalyzer.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Analyzer/Resolve/IdentifierResolver.cpp b/src/Analyzer/Resolve/IdentifierResolver.cpp index 59cf82989e7..692a31b66ba 100644 --- a/src/Analyzer/Resolve/IdentifierResolver.cpp +++ b/src/Analyzer/Resolve/IdentifierResolver.cpp @@ -1129,7 +1129,7 @@ QueryTreeNodePtr IdentifierResolver::tryResolveIdentifierFromJoin(const Identifi resolved_identifier = left_resolved_identifier; } } - else if (scope.joins_count && scope.context->getSettingsRef().single_join_prefer_left_table) + else if (scope.joins_count == 1 && scope.context->getSettingsRef().single_join_prefer_left_table) { resolved_side = JoinTableSide::Left; resolved_identifier = left_resolved_identifier; diff --git a/src/Analyzer/Resolve/QueryAnalyzer.cpp b/src/Analyzer/Resolve/QueryAnalyzer.cpp index fc7e56c281f..134b2759c93 100644 --- a/src/Analyzer/Resolve/QueryAnalyzer.cpp +++ b/src/Analyzer/Resolve/QueryAnalyzer.cpp @@ -4459,7 +4459,7 @@ void QueryAnalyzer::initializeTableExpressionData(const QueryTreeNodePtr & table { auto left_table_expression = extractLeftTableExpression(scope_query_node->getJoinTree()); if (table_expression_node.get() == left_table_expression.get() && - scope.joins_count && scope.context->getSettingsRef().single_join_prefer_left_table) + scope.joins_count == 1 && scope.context->getSettingsRef().single_join_prefer_left_table) table_expression_data.should_qualify_columns = false; } From 1f0c6061556cd64225a58933177df0228b4c185a Mon Sep 17 00:00:00 2001 From: Yakov Olkhovskiy Date: Fri, 5 Jul 2024 18:48:01 +0000 Subject: [PATCH 7/8] fix tests --- .../02378_analyzer_projection_names.reference | 4 ++-- ...056_analyzer_double_subquery_alias.reference | 1 - .../03056_analyzer_double_subquery_alias.sql | 17 ----------------- 3 files changed, 2 insertions(+), 20 deletions(-) delete mode 100644 tests/queries/0_stateless/03056_analyzer_double_subquery_alias.reference delete mode 100644 tests/queries/0_stateless/03056_analyzer_double_subquery_alias.sql diff --git a/tests/queries/0_stateless/02378_analyzer_projection_names.reference b/tests/queries/0_stateless/02378_analyzer_projection_names.reference index 170f3e9e1ad..532414f117c 100644 --- a/tests/queries/0_stateless/02378_analyzer_projection_names.reference +++ b/tests/queries/0_stateless/02378_analyzer_projection_names.reference @@ -447,13 +447,13 @@ SELECT '--'; -- DESCRIBE (WITH test_table_in_cte_1 AS (SELECT 1 AS c1), test_table_in_cte_2 AS (SELECT 1 AS c1) SELECT * FROM test_table_in_cte_1 INNER JOIN test_table_in_cte_2 as test_table_in_cte_2 ON test_table_in_cte_1.c1 = test_table_in_cte_2.c1); -c1 UInt8 +test_table_in_cte_1.c1 UInt8 test_table_in_cte_2.c1 UInt8 SELECT '--'; -- DESCRIBE (WITH test_table_in_cte_1 AS (SELECT 1 AS c1), test_table_in_cte_2 AS (SELECT 1 AS c1 UNION ALL SELECT 1 AS c1) SELECT * FROM test_table_in_cte_1 INNER JOIN test_table_in_cte_2 as test_table_in_cte_2 ON test_table_in_cte_1.c1 = test_table_in_cte_2.c1); -c1 UInt8 +test_table_in_cte_1.c1 UInt8 test_table_in_cte_2.c1 UInt8 SELECT 'Joins'; Joins diff --git a/tests/queries/0_stateless/03056_analyzer_double_subquery_alias.reference b/tests/queries/0_stateless/03056_analyzer_double_subquery_alias.reference deleted file mode 100644 index 72749c905a3..00000000000 --- a/tests/queries/0_stateless/03056_analyzer_double_subquery_alias.reference +++ /dev/null @@ -1 +0,0 @@ -1 1 1 diff --git a/tests/queries/0_stateless/03056_analyzer_double_subquery_alias.sql b/tests/queries/0_stateless/03056_analyzer_double_subquery_alias.sql deleted file mode 100644 index de471c1a091..00000000000 --- a/tests/queries/0_stateless/03056_analyzer_double_subquery_alias.sql +++ /dev/null @@ -1,17 +0,0 @@ --- https://github.com/ClickHouse/ClickHouse/issues/22627 -SET allow_experimental_analyzer=1; -WITH - x AS - ( - SELECT 1 AS a - ), - xx AS - ( - SELECT * - FROM x - , x AS x1 - , x AS x2 - ) -SELECT * -FROM xx -WHERE a = 1; From 89f8c577e472d0c3709220427c2da1e880eea3ad Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 15 Jul 2024 05:34:14 +0200 Subject: [PATCH 8/8] Do not fix anything --- tests/queries/0_stateless/00719_parallel_ddl_db.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/00719_parallel_ddl_db.sh b/tests/queries/0_stateless/00719_parallel_ddl_db.sh index ceba24df7e4..ac4e4eae4f2 100755 --- a/tests/queries/0_stateless/00719_parallel_ddl_db.sh +++ b/tests/queries/0_stateless/00719_parallel_ddl_db.sh @@ -5,7 +5,7 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) # shellcheck source=../shell_config.sh . "$CURDIR"/../shell_config.sh -DB_SUFFIX=$RANDOM +DB_SUFFIX=${RANDOM}${RANDOM}${RANDOM}${RANDOM} ${CLICKHOUSE_CLIENT} --query "DROP DATABASE IF EXISTS parallel_ddl_${DB_SUFFIX}" function query()