From 79140a35897e302bc5e45806811d74a4773c1c16 Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Fri, 30 Aug 2024 15:17:07 +0200 Subject: [PATCH] Support DESCRIBE for parameterized view; Fix usage in scalars --- src/Analyzer/QueryTreeBuilder.cpp | 85 +++++++++++++------ src/Analyzer/QueryTreeBuilder.h | 5 ++ src/Analyzer/Resolve/QueryAnalyzer.cpp | 12 ++- src/Analyzer/TableFunctionNode.h | 2 +- src/Interpreters/InterpreterCreateQuery.cpp | 10 +-- src/Interpreters/InterpreterDescribeQuery.cpp | 33 +++++++ .../ReplaceQueryParameterVisitor.cpp | 29 ------- .../ReplaceQueryParameterVisitor.h | 2 - src/Storages/StorageView.cpp | 9 +- ...3228_param_view_metadata_columns.reference | 2 + .../03228_param_view_metadata_columns.sql | 5 ++ 11 files changed, 122 insertions(+), 72 deletions(-) create mode 100644 tests/queries/0_stateless/03228_param_view_metadata_columns.reference create mode 100644 tests/queries/0_stateless/03228_param_view_metadata_columns.sql diff --git a/src/Analyzer/QueryTreeBuilder.cpp b/src/Analyzer/QueryTreeBuilder.cpp index 9754897d54d..ef04d3ba4e4 100644 --- a/src/Analyzer/QueryTreeBuilder.cpp +++ b/src/Analyzer/QueryTreeBuilder.cpp @@ -74,7 +74,17 @@ public: return query_tree_node; } + static QueryTreeNodePtr buildForTableFunction( + const ASTTableExpression & table_expression, + const ContextPtr & context) + { + QueryTreeBuilder builder; + return builder.buildTableFunction(table_expression, context); + } + private: + QueryTreeBuilder() = default; + QueryTreeNodePtr buildSelectOrUnionExpression(const ASTPtr & select_or_union_query, bool is_subquery, const std::string & cte_name, @@ -109,6 +119,11 @@ private: QueryTreeNodePtr buildJoinTree(const ASTPtr & tables_in_select_query, const ContextPtr & context) const; + QueryTreeNodePtr buildTableFunction( + const ASTTableExpression & table_expression, + const ContextPtr & context, + const std::optional & table_expression_modifiers = {}) const; + ColumnTransformersNodes buildColumnTransformers(const ASTPtr & matcher_expression, const ContextPtr & context) const; ASTPtr query; @@ -854,34 +869,7 @@ QueryTreeNodePtr QueryTreeBuilder::buildJoinTree(const ASTPtr & tables_in_select } else if (table_expression.table_function) { - auto & table_function_expression = table_expression.table_function->as(); - - auto node = std::make_shared(table_function_expression.name); - - if (table_function_expression.arguments) - { - const auto & function_arguments_list = table_function_expression.arguments->as().children; - for (const auto & argument : function_arguments_list) - { - if (!node->getSettingsChanges().empty()) - throw Exception(ErrorCodes::LOGICAL_ERROR, "Table function '{}' has arguments after SETTINGS", - table_function_expression.formatForErrorMessage()); - - if (argument->as() || argument->as() || argument->as()) - node->getArguments().getNodes().push_back(buildSelectOrUnionExpression(argument, false /*is_subquery*/, {} /*cte_name*/, context)); - else if (const auto * ast_set = argument->as()) - node->setSettingsChanges(ast_set->changes); - else - node->getArguments().getNodes().push_back(buildExpression(argument, context)); - } - } - - if (table_expression_modifiers) - node->setTableExpressionModifiers(*table_expression_modifiers); - node->setAlias(table_function_expression.tryGetAlias()); - node->setOriginalAST(table_expression.table_function); - - table_expressions.push_back(std::move(node)); + table_expressions.push_back(buildTableFunction(table_expression, context, table_expression_modifiers)); } else { @@ -983,6 +971,42 @@ QueryTreeNodePtr QueryTreeBuilder::buildJoinTree(const ASTPtr & tables_in_select } +QueryTreeNodePtr QueryTreeBuilder::buildTableFunction( + const ASTTableExpression & table_expression, + const ContextPtr & context, + const std::optional & table_expression_modifiers) const +{ + auto & table_function_expression = table_expression.table_function->as(); + + auto node = std::make_shared(table_function_expression.name); + + if (table_function_expression.arguments) + { + const auto & function_arguments_list = table_function_expression.arguments->as().children; + for (const auto & argument : function_arguments_list) + { + if (!node->getSettingsChanges().empty()) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Table function '{}' has arguments after SETTINGS", + table_function_expression.formatForErrorMessage()); + + if (argument->as() || argument->as() || argument->as()) + node->getArguments().getNodes().push_back(buildSelectOrUnionExpression(argument, false /*is_subquery*/, {} /*cte_name*/, context)); + else if (const auto * ast_set = argument->as()) + node->setSettingsChanges(ast_set->changes); + else + node->getArguments().getNodes().push_back(buildExpression(argument, context)); + } + } + + if (table_expression_modifiers) + node->setTableExpressionModifiers(*table_expression_modifiers); + node->setAlias(table_function_expression.tryGetAlias()); + node->setOriginalAST(table_expression.table_function); + + return node; +} + + ColumnTransformersNodes QueryTreeBuilder::buildColumnTransformers(const ASTPtr & matcher_expression, const ContextPtr & context) const { ColumnTransformersNodes column_transformers; @@ -1056,4 +1080,9 @@ QueryTreeNodePtr buildQueryTree(ASTPtr query, ContextPtr context) return builder.getQueryTreeNode(); } +QueryTreeNodePtr buildQueryTreeForTableFunction(const ASTTableExpression & table_expression, ContextPtr context) +{ + return QueryTreeBuilder::buildForTableFunction(table_expression, context); +} + } diff --git a/src/Analyzer/QueryTreeBuilder.h b/src/Analyzer/QueryTreeBuilder.h index acff62e07c9..5d0620a7e16 100644 --- a/src/Analyzer/QueryTreeBuilder.h +++ b/src/Analyzer/QueryTreeBuilder.h @@ -9,6 +9,8 @@ namespace DB { +struct ASTTableExpression; + /** Build query tree from AST. * AST that represent query ASTSelectWithUnionQuery, ASTSelectIntersectExceptQuery, ASTSelectQuery. * AST that represent a list of expressions ASTExpressionList. @@ -18,4 +20,7 @@ namespace DB */ QueryTreeNodePtr buildQueryTree(ASTPtr query, ContextPtr context); +// Build query tree from AST of table function. +QueryTreeNodePtr buildQueryTreeForTableFunction(const ASTTableExpression & table_expression, ContextPtr context); + } diff --git a/src/Analyzer/Resolve/QueryAnalyzer.cpp b/src/Analyzer/Resolve/QueryAnalyzer.cpp index a18c2901a58..dba9cd31c81 100644 --- a/src/Analyzer/Resolve/QueryAnalyzer.cpp +++ b/src/Analyzer/Resolve/QueryAnalyzer.cpp @@ -64,6 +64,8 @@ #include #include +#include + #include #include @@ -515,7 +517,9 @@ void QueryAnalyzer::evaluateScalarSubqueryIfNeeded(QueryTreeNodePtr & node, Iden auto options = SelectQueryOptions(QueryProcessingStage::Complete, scope.subquery_depth, true /*is_subquery*/); options.only_analyze = only_analyze; - auto interpreter = std::make_unique(node->toAST(), subquery_context, subquery_context->getViewSource(), options); + auto subquery = node->clone(); + createUniqueTableAliases(subquery, {}, subquery_context); + auto interpreter = std::make_unique(subquery->toAST(), subquery_context, subquery_context->getViewSource(), options); if (only_analyze) { @@ -4566,9 +4570,9 @@ void QueryAnalyzer::resolveTableFunction(QueryTreeNodePtr & table_function_node, if (parametrized_view_storage) { - auto fake_table_node = std::make_shared(parametrized_view_storage, scope_context); - fake_table_node->setAlias(table_function_node->getAlias()); - table_function_node = fake_table_node; + std::vector skip_analysis_arguments_indexes(table_function_node_typed.getArguments().getNodes().size()); + std::iota(skip_analysis_arguments_indexes.begin(), skip_analysis_arguments_indexes.end(), 0); + table_function_node_typed.resolve({}, parametrized_view_storage, scope_context, std::move(skip_analysis_arguments_indexes)); return; } diff --git a/src/Analyzer/TableFunctionNode.h b/src/Analyzer/TableFunctionNode.h index 98121ef95c5..6844f76fdce 100644 --- a/src/Analyzer/TableFunctionNode.h +++ b/src/Analyzer/TableFunctionNode.h @@ -73,7 +73,7 @@ public: /// Returns true, if table function is resolved, false otherwise bool isResolved() const { - return storage != nullptr && table_function != nullptr; + return storage != nullptr; } /// Get table function, returns nullptr if table function node is not resolved diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index 135a0adbb13..e9f40bdbaf5 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -845,22 +845,18 @@ InterpreterCreateQuery::TableProperties InterpreterCreateQuery::getTableProperti } else if (create.select) { - ASTPtr query = create.select->clone(); - if (create.isParameterizedView()) - { - replaceQueryParametersWithDefaults(query); - } + return properties; Block as_select_sample; if (getContext()->getSettingsRef().allow_experimental_analyzer) { - as_select_sample = InterpreterSelectQueryAnalyzer::getSampleBlock(query, getContext()); + as_select_sample = InterpreterSelectQueryAnalyzer::getSampleBlock(create.select->clone(), getContext()); } else { - as_select_sample = InterpreterSelectWithUnionQuery::getSampleBlock(query, getContext()); + as_select_sample = InterpreterSelectWithUnionQuery::getSampleBlock(create.select->clone(), getContext()); } properties.columns = ColumnsDescription(as_select_sample.getNamesAndTypesList()); diff --git a/src/Interpreters/InterpreterDescribeQuery.cpp b/src/Interpreters/InterpreterDescribeQuery.cpp index 39fc85a5e23..5a5ec329b88 100644 --- a/src/Interpreters/InterpreterDescribeQuery.cpp +++ b/src/Interpreters/InterpreterDescribeQuery.cpp @@ -4,6 +4,10 @@ #include #include #include +#include +#include +#include +#include #include #include #include @@ -146,6 +150,35 @@ void InterpreterDescribeQuery::fillColumnsFromSubquery(const ASTTableExpression void InterpreterDescribeQuery::fillColumnsFromTableFunction(const ASTTableExpression & table_expression) { auto current_context = getContext(); + if (current_context->getSettingsRef().allow_experimental_analyzer) + { + auto query_tree = buildQueryTreeForTableFunction(table_expression, current_context); + + QueryAnalysisPass query_analysis_pass(true); + query_analysis_pass.run(query_tree, current_context); + + StoragePtr storage; + if (auto * table_function_node = query_tree->as()) + storage = table_function_node->getStorage(); + else + storage = query_tree->as().getStorage(); + + auto column_descriptions = storage->getInMemoryMetadata().getColumns(); + for (const auto & column : column_descriptions) + columns.emplace_back(column); + + if (settings.describe_include_virtual_columns) + { + auto virtuals = storage->getVirtualsPtr(); + for (const auto & column : *virtuals) + { + if (!column_descriptions.has(column.name)) + virtual_columns.push_back(column); + } + } + return; + } + TableFunctionPtr table_function_ptr = TableFunctionFactory::instance().get(table_expression.table_function, current_context); auto column_descriptions = table_function_ptr->getActualTableStructure(getContext(), /*is_insert_query*/ true); diff --git a/src/Interpreters/ReplaceQueryParameterVisitor.cpp b/src/Interpreters/ReplaceQueryParameterVisitor.cpp index a5af86c1be7..46dcc6129bc 100644 --- a/src/Interpreters/ReplaceQueryParameterVisitor.cpp +++ b/src/Interpreters/ReplaceQueryParameterVisitor.cpp @@ -156,33 +156,4 @@ void ReplaceQueryParameterVisitor::visitIdentifier(ASTPtr & ast) ast_identifier->children.clear(); } -void replaceQueryParametersWithDefaults(ASTPtr & ast) -{ - std::vector nodes_to_process{ ast }; - - while (!nodes_to_process.empty()) - { - auto node = nodes_to_process.back(); - nodes_to_process.pop_back(); - for (auto & child : node->children) - { - if (auto * query_param = child->as()) - { - const auto data_type = DataTypeFactory::instance().get(query_param->type); - auto * old_ptr = child.get(); - - Field literal = data_type->getDefault(); - if (typeid_cast(data_type.get())) - child = std::make_shared(literal); - else - child = addTypeConversionToAST(std::make_shared(literal), query_param->type); - - node->updatePointerToChild(old_ptr, child.get()); - } - else - nodes_to_process.push_back(child); - } - } -} - } diff --git a/src/Interpreters/ReplaceQueryParameterVisitor.h b/src/Interpreters/ReplaceQueryParameterVisitor.h index 653615ff54e..7d5da7ea85b 100644 --- a/src/Interpreters/ReplaceQueryParameterVisitor.h +++ b/src/Interpreters/ReplaceQueryParameterVisitor.h @@ -32,6 +32,4 @@ private: void visitChildren(ASTPtr & ast); }; -void replaceQueryParametersWithDefaults(ASTPtr & ast); - } diff --git a/src/Storages/StorageView.cpp b/src/Storages/StorageView.cpp index 0be62dcae3a..878998ebf12 100644 --- a/src/Storages/StorageView.cpp +++ b/src/Storages/StorageView.cpp @@ -116,7 +116,14 @@ StorageView::StorageView( : IStorage(table_id_) { StorageInMemoryMetadata storage_metadata; - storage_metadata.setColumns(columns_); + if (!is_parameterized_view_) + { + /// If CREATE query is to create parameterized view, then we dont want to set columns + if (!query.isParameterizedView()) + storage_metadata.setColumns(columns_); + } + else + storage_metadata.setColumns(columns_); storage_metadata.setComment(comment); if (query.sql_security) diff --git a/tests/queries/0_stateless/03228_param_view_metadata_columns.reference b/tests/queries/0_stateless/03228_param_view_metadata_columns.reference new file mode 100644 index 00000000000..3d3d42b41c3 --- /dev/null +++ b/tests/queries/0_stateless/03228_param_view_metadata_columns.reference @@ -0,0 +1,2 @@ +number UInt64 +55 diff --git a/tests/queries/0_stateless/03228_param_view_metadata_columns.sql b/tests/queries/0_stateless/03228_param_view_metadata_columns.sql new file mode 100644 index 00000000000..ec3979937e9 --- /dev/null +++ b/tests/queries/0_stateless/03228_param_view_metadata_columns.sql @@ -0,0 +1,5 @@ +create view paramview as select * from system.numbers where number <= {top:UInt64}; + +describe paramview(top = 10); + +select arrayReduce('sum', (select groupArray(number) from paramview(top=10)));