From bb1058e2b28401b8189c2d21db9476fca19286cb Mon Sep 17 00:00:00 2001 From: Yakov Olkhovskiy Date: Thu, 18 Jan 2024 17:59:23 +0000 Subject: [PATCH 1/6] fix storage replacement with insertion block for analyzer --- src/Analyzer/Utils.cpp | 66 ++++++++++++++++- src/Analyzer/Utils.h | 3 + .../InterpreterSelectQueryAnalyzer.cpp | 74 ++++++------------- 3 files changed, 91 insertions(+), 52 deletions(-) diff --git a/src/Analyzer/Utils.cpp b/src/Analyzer/Utils.cpp index 53fcf534f645..af40b7f766fc 100644 --- a/src/Analyzer/Utils.cpp +++ b/src/Analyzer/Utils.cpp @@ -326,6 +326,69 @@ void addTableExpressionOrJoinIntoTablesInSelectQuery(ASTPtr & tables_in_select_q } } +QueryTreeNodes extractTrueTableExpressions(const QueryTreeNodePtr & tree) +{ + QueryTreeNodes result; + + std::deque nodes_to_process; + nodes_to_process.push_back(tree); + + while (!nodes_to_process.empty()) + { + auto node_to_process = std::move(nodes_to_process.front()); + nodes_to_process.pop_front(); + + auto node_type = node_to_process->getNodeType(); + + switch (node_type) + { + case QueryTreeNodeType::TABLE: + { + result.push_back(std::move(node_to_process)); + break; + } + case QueryTreeNodeType::QUERY: + { + nodes_to_process.push_front(node_to_process->as()->getJoinTree()); + break; + } + case QueryTreeNodeType::UNION: + { + for (auto union_node : node_to_process->as()->getQueries().getNodes()) + nodes_to_process.push_front(union_node); + break; + } + case QueryTreeNodeType::TABLE_FUNCTION: + { + for (auto argument_node : node_to_process->as()->getArgumentsNode()->getChildren()) + nodes_to_process.push_front(argument_node); + break; + } + case QueryTreeNodeType::ARRAY_JOIN: + { + nodes_to_process.push_front(node_to_process->as()->getTableExpression()); + break; + } + case QueryTreeNodeType::JOIN: + { + auto & join_node = node_to_process->as(); + nodes_to_process.push_front(join_node.getRightTableExpression()); + nodes_to_process.push_front(join_node.getLeftTableExpression()); + break; + } + default: + { + throw Exception(ErrorCodes::LOGICAL_ERROR, + "Unexpected node type for table expression. " + "Expected table, table function, query, union, join or array join. Actual {}", + node_to_process->getNodeTypeName()); + } + } + } + + return result; +} + QueryTreeNodes extractTableExpressions(const QueryTreeNodePtr & join_tree_node, bool add_array_join) { QueryTreeNodes result; @@ -383,6 +446,7 @@ QueryTreeNodes extractTableExpressions(const QueryTreeNodePtr & join_tree_node, QueryTreeNodePtr extractLeftTableExpression(const QueryTreeNodePtr & join_tree_node) { +std::cout << "\033[1;31m" << "+++ JOO extractLeftTableExpression 0" << "\033[0m" << std::endl; QueryTreeNodePtr result; std::deque nodes_to_process; @@ -394,7 +458,7 @@ QueryTreeNodePtr extractLeftTableExpression(const QueryTreeNodePtr & join_tree_n nodes_to_process.pop_front(); auto node_type = node_to_process->getNodeType(); - +std::cout << "\033[1;31m" << "+++ JOO extractLeftTableExpression " << static_cast(node_type) << "\033[0m" << std::endl; switch (node_type) { case QueryTreeNodeType::TABLE: diff --git a/src/Analyzer/Utils.h b/src/Analyzer/Utils.h index d3eb6ba3cc26..04a5e4609744 100644 --- a/src/Analyzer/Utils.h +++ b/src/Analyzer/Utils.h @@ -50,6 +50,9 @@ std::optional tryExtractConstantFromConditionNode(const QueryTreeNodePtr & */ void addTableExpressionOrJoinIntoTablesInSelectQuery(ASTPtr & tables_in_select_query_ast, const QueryTreeNodePtr & table_expression, const IQueryTreeNode::ConvertToASTOptions & convert_to_ast_options); +/// Extract table expressions from tree +QueryTreeNodes extractTrueTableExpressions(const QueryTreeNodePtr & tree); + /// Extract table, table function, query, union from join tree QueryTreeNodes extractTableExpressions(const QueryTreeNodePtr & join_tree_node, bool add_array_join = false); diff --git a/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp b/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp index 868ef170f7ce..a44ff1a79df5 100644 --- a/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp +++ b/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp @@ -1,3 +1,5 @@ +#include "Analyzer/IQueryTreeNode.h" +#include "Analyzer/JoinNode.h" #include #include @@ -74,60 +76,28 @@ ContextMutablePtr buildContext(const ContextPtr & context, const SelectQueryOpti void replaceStorageInQueryTree(QueryTreeNodePtr & query_tree, const ContextPtr & context, const StoragePtr & storage) { - auto query_to_replace_table_expression = query_tree; - QueryTreeNodePtr table_expression_to_replace; - - while (!table_expression_to_replace) + auto nodes = extractTrueTableExpressions(query_tree); + IQueryTreeNode::ReplacementMap replacement_map; + + for (auto & node : nodes) { - if (auto * union_node = query_to_replace_table_expression->as()) - query_to_replace_table_expression = union_node->getQueries().getNodes().at(0); - - auto & query_to_replace_table_expression_typed = query_to_replace_table_expression->as(); - auto left_table_expression = extractLeftTableExpression(query_to_replace_table_expression_typed.getJoinTree()); - auto left_table_expression_node_type = left_table_expression->getNodeType(); - - switch (left_table_expression_node_type) - { - case QueryTreeNodeType::QUERY: - case QueryTreeNodeType::UNION: - { - query_to_replace_table_expression = std::move(left_table_expression); - break; - } - case QueryTreeNodeType::TABLE: - case QueryTreeNodeType::TABLE_FUNCTION: - case QueryTreeNodeType::IDENTIFIER: - { - table_expression_to_replace = std::move(left_table_expression); - break; - } - default: - { - throw Exception(ErrorCodes::UNSUPPORTED_METHOD, - "Expected table, table function or identifier node to replace with storage. Actual {}", - left_table_expression->formatASTForErrorMessage()); - } - } - } + auto & table_node = node->as(); - /// Don't replace storage if table name differs - if (auto * table_node = table_expression_to_replace->as(); table_node && table_node->getStorageID().getFullNameNotQuoted() != storage->getStorageID().getFullNameNotQuoted()) - return; + /// Don't replace storage if table name differs + if (table_node.getStorageID().getFullNameNotQuoted() != storage->getStorageID().getFullNameNotQuoted()) + continue; - auto replacement_table_expression = std::make_shared(storage, context); - std::optional table_expression_modifiers; + auto replacement_table_expression = std::make_shared(storage, context); - if (auto * table_node = table_expression_to_replace->as()) - table_expression_modifiers = table_node->getTableExpressionModifiers(); - else if (auto * table_function_node = table_expression_to_replace->as()) - table_expression_modifiers = table_function_node->getTableExpressionModifiers(); - else if (auto * identifier_node = table_expression_to_replace->as()) - table_expression_modifiers = identifier_node->getTableExpressionModifiers(); + if (auto table_expression_modifiers = table_node.getTableExpressionModifiers()) + replacement_table_expression->setTableExpressionModifiers(*table_expression_modifiers); - if (table_expression_modifiers) - replacement_table_expression->setTableExpressionModifiers(*table_expression_modifiers); + if (node->hasAlias()) + replacement_table_expression->setAlias(node->getAlias() + "_replacement"); - query_tree = query_tree->cloneAndReplace(table_expression_to_replace, std::move(replacement_table_expression)); + replacement_map.emplace(node.get(), std::move(replacement_table_expression)); + } + query_tree = query_tree->cloneAndReplace(replacement_map); } QueryTreeNodePtr buildQueryTreeAndRunPasses(const ASTPtr & query, @@ -147,10 +117,12 @@ QueryTreeNodePtr buildQueryTreeAndRunPasses(const ASTPtr & query, query_tree_pass_manager.runOnlyResolve(query_tree); else query_tree_pass_manager.run(query_tree); - - if (storage) +std::cout << "\033[1;31m" << "+++ JOO query_tree\n" << query_tree->dumpTree() << "\033[0m" << std::endl; + if (storage) { replaceStorageInQueryTree(query_tree, context, storage); - +// replaceStorageInQueryTree(query_tree, context, storage, false); + } +std::cout << "\033[1;31m" << "+++ JOO query_tree NEW\n" << query_tree->dumpTree() << "\033[0m" << std::endl; return query_tree; } From b914938c4ca27a3eb41fe6f4b5d0c6de2aff3f2a Mon Sep 17 00:00:00 2001 From: Yakov Olkhovskiy Date: Thu, 18 Jan 2024 18:10:54 +0000 Subject: [PATCH 2/6] clenup --- src/Analyzer/Utils.cpp | 3 +-- src/Interpreters/InterpreterSelectQueryAnalyzer.cpp | 10 ++++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/Analyzer/Utils.cpp b/src/Analyzer/Utils.cpp index af40b7f766fc..26067141b1dd 100644 --- a/src/Analyzer/Utils.cpp +++ b/src/Analyzer/Utils.cpp @@ -446,7 +446,6 @@ QueryTreeNodes extractTableExpressions(const QueryTreeNodePtr & join_tree_node, QueryTreeNodePtr extractLeftTableExpression(const QueryTreeNodePtr & join_tree_node) { -std::cout << "\033[1;31m" << "+++ JOO extractLeftTableExpression 0" << "\033[0m" << std::endl; QueryTreeNodePtr result; std::deque nodes_to_process; @@ -458,7 +457,7 @@ std::cout << "\033[1;31m" << "+++ JOO extractLeftTableExpression 0" << "\033[0m" nodes_to_process.pop_front(); auto node_type = node_to_process->getNodeType(); -std::cout << "\033[1;31m" << "+++ JOO extractLeftTableExpression " << static_cast(node_type) << "\033[0m" << std::endl; + switch (node_type) { case QueryTreeNodeType::TABLE: diff --git a/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp b/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp index a44ff1a79df5..0aeafb8a9798 100644 --- a/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp +++ b/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp @@ -78,7 +78,7 @@ void replaceStorageInQueryTree(QueryTreeNodePtr & query_tree, const ContextPtr & { auto nodes = extractTrueTableExpressions(query_tree); IQueryTreeNode::ReplacementMap replacement_map; - + for (auto & node : nodes) { auto & table_node = node->as(); @@ -117,12 +117,10 @@ QueryTreeNodePtr buildQueryTreeAndRunPasses(const ASTPtr & query, query_tree_pass_manager.runOnlyResolve(query_tree); else query_tree_pass_manager.run(query_tree); -std::cout << "\033[1;31m" << "+++ JOO query_tree\n" << query_tree->dumpTree() << "\033[0m" << std::endl; - if (storage) { + + if (storage) replaceStorageInQueryTree(query_tree, context, storage); -// replaceStorageInQueryTree(query_tree, context, storage, false); - } -std::cout << "\033[1;31m" << "+++ JOO query_tree NEW\n" << query_tree->dumpTree() << "\033[0m" << std::endl; + return query_tree; } From 1144f6051519afb06d6728ce2adf694aea47c13c Mon Sep 17 00:00:00 2001 From: Yakov Olkhovskiy Date: Thu, 18 Jan 2024 18:17:07 +0000 Subject: [PATCH 3/6] clenup --- src/Interpreters/InterpreterSelectQueryAnalyzer.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp b/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp index 0aeafb8a9798..8e6d48102f3b 100644 --- a/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp +++ b/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp @@ -1,5 +1,4 @@ -#include "Analyzer/IQueryTreeNode.h" -#include "Analyzer/JoinNode.h" +#include #include #include From 43aaccdaa165ef6c56937247c7a1bcba337f8d52 Mon Sep 17 00:00:00 2001 From: Yakov Olkhovskiy Date: Thu, 18 Jan 2024 19:17:05 +0000 Subject: [PATCH 4/6] clang tidy --- src/Analyzer/Utils.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Analyzer/Utils.cpp b/src/Analyzer/Utils.cpp index 26067141b1dd..9c3a7c2fb557 100644 --- a/src/Analyzer/Utils.cpp +++ b/src/Analyzer/Utils.cpp @@ -354,13 +354,13 @@ QueryTreeNodes extractTrueTableExpressions(const QueryTreeNodePtr & tree) } case QueryTreeNodeType::UNION: { - for (auto union_node : node_to_process->as()->getQueries().getNodes()) + for (const auto & union_node : node_to_process->as()->getQueries().getNodes()) nodes_to_process.push_front(union_node); break; } case QueryTreeNodeType::TABLE_FUNCTION: { - for (auto argument_node : node_to_process->as()->getArgumentsNode()->getChildren()) + for (const auto & argument_node : node_to_process->as()->getArgumentsNode()->getChildren()) nodes_to_process.push_front(argument_node); break; } From 7a95eb8bd1df2cb469ed5b9c9c2a8707a8abf674 Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Tue, 23 Jan 2024 11:59:41 +0100 Subject: [PATCH 5/6] Review fixes --- src/Analyzer/Utils.cpp | 20 +++++++++---------- src/Analyzer/Utils.h | 8 ++++---- .../InterpreterSelectQueryAnalyzer.cpp | 6 +----- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/Analyzer/Utils.cpp b/src/Analyzer/Utils.cpp index 9c3a7c2fb557..da6ab844dd3c 100644 --- a/src/Analyzer/Utils.cpp +++ b/src/Analyzer/Utils.cpp @@ -326,17 +326,17 @@ void addTableExpressionOrJoinIntoTablesInSelectQuery(ASTPtr & tables_in_select_q } } -QueryTreeNodes extractTrueTableExpressions(const QueryTreeNodePtr & tree) +QueryTreeNodes extractAllTableReferences(const QueryTreeNodePtr & tree) { QueryTreeNodes result; - std::deque nodes_to_process; + QueryTreeNodes nodes_to_process; nodes_to_process.push_back(tree); while (!nodes_to_process.empty()) { - auto node_to_process = std::move(nodes_to_process.front()); - nodes_to_process.pop_front(); + auto node_to_process = std::move(nodes_to_process.back()); + nodes_to_process.pop_back(); auto node_type = node_to_process->getNodeType(); @@ -349,31 +349,31 @@ QueryTreeNodes extractTrueTableExpressions(const QueryTreeNodePtr & tree) } case QueryTreeNodeType::QUERY: { - nodes_to_process.push_front(node_to_process->as()->getJoinTree()); + nodes_to_process.push_back(node_to_process->as()->getJoinTree()); break; } case QueryTreeNodeType::UNION: { for (const auto & union_node : node_to_process->as()->getQueries().getNodes()) - nodes_to_process.push_front(union_node); + nodes_to_process.push_back(union_node); break; } case QueryTreeNodeType::TABLE_FUNCTION: { for (const auto & argument_node : node_to_process->as()->getArgumentsNode()->getChildren()) - nodes_to_process.push_front(argument_node); + nodes_to_process.push_back(argument_node); break; } case QueryTreeNodeType::ARRAY_JOIN: { - nodes_to_process.push_front(node_to_process->as()->getTableExpression()); + nodes_to_process.push_back(node_to_process->as()->getTableExpression()); break; } case QueryTreeNodeType::JOIN: { auto & join_node = node_to_process->as(); - nodes_to_process.push_front(join_node.getRightTableExpression()); - nodes_to_process.push_front(join_node.getLeftTableExpression()); + nodes_to_process.push_back(join_node.getRightTableExpression()); + nodes_to_process.push_back(join_node.getLeftTableExpression()); break; } default: diff --git a/src/Analyzer/Utils.h b/src/Analyzer/Utils.h index 04a5e4609744..121384bdc7ce 100644 --- a/src/Analyzer/Utils.h +++ b/src/Analyzer/Utils.h @@ -50,13 +50,13 @@ std::optional tryExtractConstantFromConditionNode(const QueryTreeNodePtr & */ void addTableExpressionOrJoinIntoTablesInSelectQuery(ASTPtr & tables_in_select_query_ast, const QueryTreeNodePtr & table_expression, const IQueryTreeNode::ConvertToASTOptions & convert_to_ast_options); -/// Extract table expressions from tree -QueryTreeNodes extractTrueTableExpressions(const QueryTreeNodePtr & tree); +/// Extract all TableNodes from the query tree. +QueryTreeNodes extractAllTableReferences(const QueryTreeNodePtr & tree); -/// Extract table, table function, query, union from join tree +/// Extract table, table function, query, union from join tree. QueryTreeNodes extractTableExpressions(const QueryTreeNodePtr & join_tree_node, bool add_array_join = false); -/// Extract left table expression from join tree +/// Extract left table expression from join tree. QueryTreeNodePtr extractLeftTableExpression(const QueryTreeNodePtr & join_tree_node); /** Build table expressions stack that consists from table, table function, query, union, join, array join from join tree. diff --git a/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp b/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp index 8e6d48102f3b..4897101d80b2 100644 --- a/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp +++ b/src/Interpreters/InterpreterSelectQueryAnalyzer.cpp @@ -1,4 +1,3 @@ -#include #include #include @@ -75,7 +74,7 @@ ContextMutablePtr buildContext(const ContextPtr & context, const SelectQueryOpti void replaceStorageInQueryTree(QueryTreeNodePtr & query_tree, const ContextPtr & context, const StoragePtr & storage) { - auto nodes = extractTrueTableExpressions(query_tree); + auto nodes = extractAllTableReferences(query_tree); IQueryTreeNode::ReplacementMap replacement_map; for (auto & node : nodes) @@ -91,9 +90,6 @@ void replaceStorageInQueryTree(QueryTreeNodePtr & query_tree, const ContextPtr & if (auto table_expression_modifiers = table_node.getTableExpressionModifiers()) replacement_table_expression->setTableExpressionModifiers(*table_expression_modifiers); - if (node->hasAlias()) - replacement_table_expression->setAlias(node->getAlias() + "_replacement"); - replacement_map.emplace(node.get(), std::move(replacement_table_expression)); } query_tree = query_tree->cloneAndReplace(replacement_map); From 7826706ff923f2ef85d2f6fd02c539a29e7b3421 Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Wed, 24 Jan 2024 16:09:26 +0100 Subject: [PATCH 6/6] Small fix --- src/Analyzer/Utils.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Analyzer/Utils.cpp b/src/Analyzer/Utils.cpp index da6ab844dd3c..1315af85d4c1 100644 --- a/src/Analyzer/Utils.cpp +++ b/src/Analyzer/Utils.cpp @@ -360,8 +360,7 @@ QueryTreeNodes extractAllTableReferences(const QueryTreeNodePtr & tree) } case QueryTreeNodeType::TABLE_FUNCTION: { - for (const auto & argument_node : node_to_process->as()->getArgumentsNode()->getChildren()) - nodes_to_process.push_back(argument_node); + // Arguments of table function can't contain TableNodes. break; } case QueryTreeNodeType::ARRAY_JOIN: