From 967ba9d3d462e233428ae522d200051738d3791f Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Tue, 27 Aug 2024 20:20:36 +0200 Subject: [PATCH 01/10] Parameterized views: Analyze SELECT query --- src/Interpreters/InterpreterCreateQuery.cpp | 10 +++++-- .../ReplaceQueryParameterVisitor.cpp | 29 +++++++++++++++++++ .../ReplaceQueryParameterVisitor.h | 2 ++ src/Storages/StorageView.cpp | 9 +----- 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index e9f40bdbaf5..135a0adbb13 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -845,18 +845,22 @@ InterpreterCreateQuery::TableProperties InterpreterCreateQuery::getTableProperti } else if (create.select) { + ASTPtr query = create.select->clone(); + if (create.isParameterizedView()) - return properties; + { + replaceQueryParametersWithDefaults(query); + } Block as_select_sample; if (getContext()->getSettingsRef().allow_experimental_analyzer) { - as_select_sample = InterpreterSelectQueryAnalyzer::getSampleBlock(create.select->clone(), getContext()); + as_select_sample = InterpreterSelectQueryAnalyzer::getSampleBlock(query, getContext()); } else { - as_select_sample = InterpreterSelectWithUnionQuery::getSampleBlock(create.select->clone(), getContext()); + as_select_sample = InterpreterSelectWithUnionQuery::getSampleBlock(query, getContext()); } properties.columns = ColumnsDescription(as_select_sample.getNamesAndTypesList()); diff --git a/src/Interpreters/ReplaceQueryParameterVisitor.cpp b/src/Interpreters/ReplaceQueryParameterVisitor.cpp index 46dcc6129bc..a5af86c1be7 100644 --- a/src/Interpreters/ReplaceQueryParameterVisitor.cpp +++ b/src/Interpreters/ReplaceQueryParameterVisitor.cpp @@ -156,4 +156,33 @@ 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 7d5da7ea85b..653615ff54e 100644 --- a/src/Interpreters/ReplaceQueryParameterVisitor.h +++ b/src/Interpreters/ReplaceQueryParameterVisitor.h @@ -32,4 +32,6 @@ private: void visitChildren(ASTPtr & ast); }; +void replaceQueryParametersWithDefaults(ASTPtr & ast); + } diff --git a/src/Storages/StorageView.cpp b/src/Storages/StorageView.cpp index 878998ebf12..0be62dcae3a 100644 --- a/src/Storages/StorageView.cpp +++ b/src/Storages/StorageView.cpp @@ -116,14 +116,7 @@ StorageView::StorageView( : IStorage(table_id_) { StorageInMemoryMetadata storage_metadata; - 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.setColumns(columns_); storage_metadata.setComment(comment); if (query.sql_security) From 79140a35897e302bc5e45806811d74a4773c1c16 Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Fri, 30 Aug 2024 15:17:07 +0200 Subject: [PATCH 02/10] 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))); From f4c4b58e64c7cbd01a50c49632e4fa05795fece0 Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Mon, 2 Sep 2024 14:55:23 +0200 Subject: [PATCH 03/10] Rewrite DESCRIBE for parameterized view implementation --- src/Analyzer/Resolve/QueryAnalyzer.cpp | 1 - src/Analyzer/Utils.cpp | 19 +++++ src/Analyzer/Utils.h | 1 + src/Interpreters/InterpreterDescribeQuery.cpp | 73 +++++++++++++------ src/Interpreters/InterpreterDescribeQuery.h | 2 + ...3228_param_view_metadata_columns.reference | 1 + .../03228_param_view_metadata_columns.sql | 4 + 7 files changed, 76 insertions(+), 25 deletions(-) diff --git a/src/Analyzer/Resolve/QueryAnalyzer.cpp b/src/Analyzer/Resolve/QueryAnalyzer.cpp index dba9cd31c81..0d5f02586b6 100644 --- a/src/Analyzer/Resolve/QueryAnalyzer.cpp +++ b/src/Analyzer/Resolve/QueryAnalyzer.cpp @@ -4518,7 +4518,6 @@ void QueryAnalyzer::resolveTableFunction(QueryTreeNodePtr & table_function_node, String database_name = scope_context->getCurrentDatabase(); String table_name; - auto function_ast = table_function_node->toAST(); Identifier table_identifier{table_function_name}; if (table_identifier.getPartsSize() == 1) { diff --git a/src/Analyzer/Utils.cpp b/src/Analyzer/Utils.cpp index e5f372b7368..33686c338c1 100644 --- a/src/Analyzer/Utils.cpp +++ b/src/Analyzer/Utils.cpp @@ -938,4 +938,23 @@ QueryTreeNodePtr buildSubqueryToReadColumnsFromTableExpression(const QueryTreeNo return buildSubqueryToReadColumnsFromTableExpression(columns_to_select, table_node, context); } +std::pair extractDatabaseAndTableNameForParametrizedView(const String & table_function_name, const ContextPtr & context) +{ + String database_name = context->getCurrentDatabase(); + String table_name; + + Identifier table_identifier{table_function_name}; + if (table_identifier.getPartsSize() == 1) + { + table_name = table_identifier[0]; + } + else if (table_identifier.getPartsSize() == 2) + { + database_name = table_identifier[0]; + table_name = table_identifier[1]; + } + + return { database_name, table_name }; +} + } diff --git a/src/Analyzer/Utils.h b/src/Analyzer/Utils.h index f2e2c500384..a2259616ff1 100644 --- a/src/Analyzer/Utils.h +++ b/src/Analyzer/Utils.h @@ -159,5 +159,6 @@ QueryTreeNodePtr buildSubqueryToReadColumnsFromTableExpression(const NamesAndTyp */ QueryTreeNodePtr buildSubqueryToReadColumnsFromTableExpression(const QueryTreeNodePtr & table_node, const ContextPtr & context); +std::pair extractDatabaseAndTableNameForParametrizedView(const String & table_function_name, const ContextPtr & context); } diff --git a/src/Interpreters/InterpreterDescribeQuery.cpp b/src/Interpreters/InterpreterDescribeQuery.cpp index 5a5ec329b88..23b0f3ad54b 100644 --- a/src/Interpreters/InterpreterDescribeQuery.cpp +++ b/src/Interpreters/InterpreterDescribeQuery.cpp @@ -3,12 +3,15 @@ #include #include #include +#include #include +#include #include #include #include #include #include +#include #include #include #include @@ -28,6 +31,14 @@ namespace DB { +namespace ErrorCodes +{ + +extern const int UNSUPPORTED_METHOD; +extern const int UNKNOWN_FUNCTION; + +} + InterpreterDescribeQuery::InterpreterDescribeQuery(const ASTPtr & query_ptr_, ContextPtr context_) : WithContext(context_) , query_ptr(query_ptr_) @@ -129,10 +140,14 @@ BlockIO InterpreterDescribeQuery::execute() void InterpreterDescribeQuery::fillColumnsFromSubquery(const ASTTableExpression & table_expression) { - Block sample_block; auto select_query = table_expression.subquery->children.at(0); auto current_context = getContext(); + fillColumnsFromSubqueryImpl(select_query, current_context); +} +void InterpreterDescribeQuery::fillColumnsFromSubqueryImpl(const ASTPtr & select_query, const ContextPtr & current_context) +{ + Block sample_block; if (settings.allow_experimental_analyzer) { SelectQueryOptions select_query_options; @@ -150,36 +165,38 @@ void InterpreterDescribeQuery::fillColumnsFromSubquery(const ASTTableExpression void InterpreterDescribeQuery::fillColumnsFromTableFunction(const ASTTableExpression & table_expression) { auto current_context = getContext(); - if (current_context->getSettingsRef().allow_experimental_analyzer) + + auto table_function_name = table_expression.table_function->as()->name; + TableFunctionPtr table_function_ptr = TableFunctionFactory::instance().tryGet(table_function_name, current_context); + + if (!table_function_ptr) { - auto query_tree = buildQueryTreeForTableFunction(table_expression, current_context); + auto [database_name, table_name] = extractDatabaseAndTableNameForParametrizedView(table_function_name, current_context); + auto table_id = getContext()->resolveStorageID({database_name, table_name}); + getContext()->checkAccess(AccessType::SHOW_COLUMNS, table_id); + auto table = DatabaseCatalog::instance().getTable(table_id, getContext()); - 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) + if (auto * storage_view = table->as()) { - auto virtuals = storage->getVirtualsPtr(); - for (const auto & column : *virtuals) + if (storage_view->isParameterizedView()) { - if (!column_descriptions.has(column.name)) - virtual_columns.push_back(column); + auto query = storage_view->getInMemoryMetadataPtr()->getSelectQuery().inner_query->clone(); + NameToNameMap parameterized_view_values = analyzeFunctionParamValues(table_expression.table_function, current_context); + StorageView::replaceQueryParametersIfParametrizedView(query, parameterized_view_values); + return fillColumnsFromSubqueryImpl(query, current_context); } } - return; - } - TableFunctionPtr table_function_ptr = TableFunctionFactory::instance().get(table_expression.table_function, current_context); + auto hints = TableFunctionFactory::instance().getHints(table_function_name); + if (!hints.empty()) + throw Exception(ErrorCodes::UNKNOWN_FUNCTION, "Unknown table function {}. Maybe you meant: {}", table_function_name, toString(hints)); + else + throw Exception(ErrorCodes::UNKNOWN_FUNCTION, "Unknown table function {}", table_function_name); + } + else + { + table_function_ptr->parseArguments(table_expression.table_function, current_context); + } auto column_descriptions = table_function_ptr->getActualTableStructure(getContext(), /*is_insert_query*/ true); for (const auto & column : column_descriptions) @@ -205,6 +222,14 @@ void InterpreterDescribeQuery::fillColumnsFromTable(const ASTTableExpression & t auto table_id = getContext()->resolveStorageID(table_expression.database_and_table_name); getContext()->checkAccess(AccessType::SHOW_COLUMNS, table_id); auto table = DatabaseCatalog::instance().getTable(table_id, getContext()); + + if (auto * storage_view = table->as()) + { + if (storage_view->isParameterizedView()) + throw Exception(ErrorCodes::UNSUPPORTED_METHOD, + "Cannot infer table schema for the parametrized view when no query parameters are provided"); + } + auto table_lock = table->lockForShare(getContext()->getInitialQueryId(), settings.lock_acquire_timeout); auto metadata_snapshot = table->getInMemoryMetadataPtr(); diff --git a/src/Interpreters/InterpreterDescribeQuery.h b/src/Interpreters/InterpreterDescribeQuery.h index 5d01745db6b..e5e2518309b 100644 --- a/src/Interpreters/InterpreterDescribeQuery.h +++ b/src/Interpreters/InterpreterDescribeQuery.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -24,6 +25,7 @@ public: private: void fillColumnsFromSubquery(const ASTTableExpression & table_expression); + void fillColumnsFromSubqueryImpl(const ASTPtr & select_query, const ContextPtr & current_context); void fillColumnsFromTableFunction(const ASTTableExpression & table_expression); void fillColumnsFromTable(const ASTTableExpression & table_expression); diff --git a/tests/queries/0_stateless/03228_param_view_metadata_columns.reference b/tests/queries/0_stateless/03228_param_view_metadata_columns.reference index 3d3d42b41c3..c47317f0224 100644 --- a/tests/queries/0_stateless/03228_param_view_metadata_columns.reference +++ b/tests/queries/0_stateless/03228_param_view_metadata_columns.reference @@ -1,2 +1,3 @@ number UInt64 +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 index ec3979937e9..9f8e6e0d3d5 100644 --- a/tests/queries/0_stateless/03228_param_view_metadata_columns.sql +++ b/tests/queries/0_stateless/03228_param_view_metadata_columns.sql @@ -1,5 +1,9 @@ create view paramview as select * from system.numbers where number <= {top:UInt64}; +describe paramview; -- { serverError UNSUPPORTED_METHOD } + describe paramview(top = 10); +describe paramview(top = 2 + 2); + select arrayReduce('sum', (select groupArray(number) from paramview(top=10))); From 887b49649c62bd21f660ecf33a00b8448a9109c0 Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Mon, 2 Sep 2024 14:59:17 +0200 Subject: [PATCH 04/10] Small refactoring --- src/Analyzer/Resolve/QueryAnalyzer.cpp | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/Analyzer/Resolve/QueryAnalyzer.cpp b/src/Analyzer/Resolve/QueryAnalyzer.cpp index 0d5f02586b6..36e0a21356f 100644 --- a/src/Analyzer/Resolve/QueryAnalyzer.cpp +++ b/src/Analyzer/Resolve/QueryAnalyzer.cpp @@ -4515,19 +4515,7 @@ void QueryAnalyzer::resolveTableFunction(QueryTreeNodePtr & table_function_node, TableFunctionPtr table_function_ptr = TableFunctionFactory::instance().tryGet(table_function_name, scope_context); if (!table_function_ptr) { - String database_name = scope_context->getCurrentDatabase(); - String table_name; - - Identifier table_identifier{table_function_name}; - if (table_identifier.getPartsSize() == 1) - { - table_name = table_identifier[0]; - } - else if (table_identifier.getPartsSize() == 2) - { - database_name = table_identifier[0]; - table_name = table_identifier[1]; - } + auto [database_name, table_name] = extractDatabaseAndTableNameForParametrizedView(table_function_name, scope_context); /// Collect parametrized view arguments NameToNameMap view_params; From 34ceebe9920c09a28c5b5a7974bc93906b315b58 Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Mon, 2 Sep 2024 15:12:42 +0200 Subject: [PATCH 05/10] Rollback changes in QueryTreeBuilder --- src/Analyzer/QueryTreeBuilder.cpp | 85 ++++++++++--------------------- src/Analyzer/QueryTreeBuilder.h | 5 -- 2 files changed, 28 insertions(+), 62 deletions(-) diff --git a/src/Analyzer/QueryTreeBuilder.cpp b/src/Analyzer/QueryTreeBuilder.cpp index ef04d3ba4e4..9754897d54d 100644 --- a/src/Analyzer/QueryTreeBuilder.cpp +++ b/src/Analyzer/QueryTreeBuilder.cpp @@ -74,17 +74,7 @@ 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, @@ -119,11 +109,6 @@ 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; @@ -869,7 +854,34 @@ QueryTreeNodePtr QueryTreeBuilder::buildJoinTree(const ASTPtr & tables_in_select } else if (table_expression.table_function) { - table_expressions.push_back(buildTableFunction(table_expression, context, table_expression_modifiers)); + 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)); } else { @@ -971,42 +983,6 @@ 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; @@ -1080,9 +1056,4 @@ 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 5d0620a7e16..acff62e07c9 100644 --- a/src/Analyzer/QueryTreeBuilder.h +++ b/src/Analyzer/QueryTreeBuilder.h @@ -9,8 +9,6 @@ namespace DB { -struct ASTTableExpression; - /** Build query tree from AST. * AST that represent query ASTSelectWithUnionQuery, ASTSelectIntersectExceptQuery, ASTSelectQuery. * AST that represent a list of expressions ASTExpressionList. @@ -20,7 +18,4 @@ struct ASTTableExpression; */ QueryTreeNodePtr buildQueryTree(ASTPtr query, ContextPtr context); -// Build query tree from AST of table function. -QueryTreeNodePtr buildQueryTreeForTableFunction(const ASTTableExpression & table_expression, ContextPtr context); - } From 355d7bce0559d27f0e777848ed55990e0627a6c1 Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Mon, 2 Sep 2024 16:16:46 +0200 Subject: [PATCH 06/10] Improve param view docs --- docs/en/sql-reference/statements/create/view.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/en/sql-reference/statements/create/view.md b/docs/en/sql-reference/statements/create/view.md index 45e7a41e8a2..b2b58e1f4be 100644 --- a/docs/en/sql-reference/statements/create/view.md +++ b/docs/en/sql-reference/statements/create/view.md @@ -41,15 +41,24 @@ SELECT a, b, c FROM (SELECT ...) ## Parameterized View -Parametrized views are similar to normal views, but can be created with parameters which are not resolved immediately. These views can be used with table functions, which specify the name of the view as function name and the parameter values as its arguments. +Parametrized views are similar to normal views, but can be created with parameters which are not resolved immediately. +These views can be used with table functions, which specify the name of the view as function name and the parameter values as its arguments. ``` sql -CREATE VIEW view AS SELECT * FROM TABLE WHERE Column1={column1:datatype1} and Column2={column2:datatype2} ... +CREATE VIEW param_view AS SELECT * FROM TABLE WHERE Column1={column1:datatype1} and Column2={column2:datatype2} ... ``` The above creates a view for table which can be used as table function by substituting parameters as shown below. ``` sql -SELECT * FROM view(column1=value1, column2=value2 ...) +SELECT * FROM param_view(column1=value1, column2=value2 ...) +``` + +Since the parameterized view depends on the parameter values, it doesn't have a schema when parameters are not provided. +That means there's no information about parameterized views in the `system.columns` table. +Also, `DESCRIBE` queries would work only if parameters are provided. + +```sql +DESCRIBE param_view(column1=value1, column2=value2 ...) ``` ## Materialized View From 769589dbc99345791408d2c69b438f4f76007392 Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Mon, 2 Sep 2024 17:02:15 +0200 Subject: [PATCH 07/10] Remove redundant code --- src/Analyzer/Resolve/QueryAnalyzer.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Analyzer/Resolve/QueryAnalyzer.cpp b/src/Analyzer/Resolve/QueryAnalyzer.cpp index 36e0a21356f..3109157cd1c 100644 --- a/src/Analyzer/Resolve/QueryAnalyzer.cpp +++ b/src/Analyzer/Resolve/QueryAnalyzer.cpp @@ -64,8 +64,6 @@ #include #include -#include - #include #include @@ -517,9 +515,7 @@ void QueryAnalyzer::evaluateScalarSubqueryIfNeeded(QueryTreeNodePtr & node, Iden auto options = SelectQueryOptions(QueryProcessingStage::Complete, scope.subquery_depth, true /*is_subquery*/); options.only_analyze = only_analyze; - auto subquery = node->clone(); - createUniqueTableAliases(subquery, {}, subquery_context); - auto interpreter = std::make_unique(subquery->toAST(), subquery_context, subquery_context->getViewSource(), options); + auto interpreter = std::make_unique(node->toAST(), subquery_context, subquery_context->getViewSource(), options); if (only_analyze) { From 5c39591ad1b08bf5168e6edf45f5a4a741ab2248 Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Mon, 2 Sep 2024 17:07:59 +0200 Subject: [PATCH 08/10] Add tests --- .../03228_param_view_metadata_columns.reference | 5 +++++ .../03228_param_view_metadata_columns.sql | 12 ++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/queries/0_stateless/03228_param_view_metadata_columns.reference b/tests/queries/0_stateless/03228_param_view_metadata_columns.reference index c47317f0224..f1a9e1df1c8 100644 --- a/tests/queries/0_stateless/03228_param_view_metadata_columns.reference +++ b/tests/queries/0_stateless/03228_param_view_metadata_columns.reference @@ -1,3 +1,8 @@ number UInt64 number UInt64 +number UInt64 +\'Biba\' String +CAST(dummy, \'Int\') Int32 +CAST(dummy, \'String\') String +0 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 index 9f8e6e0d3d5..90e261c4445 100644 --- a/tests/queries/0_stateless/03228_param_view_metadata_columns.sql +++ b/tests/queries/0_stateless/03228_param_view_metadata_columns.sql @@ -1,9 +1,17 @@ create view paramview as select * from system.numbers where number <= {top:UInt64}; describe paramview; -- { serverError UNSUPPORTED_METHOD } - describe paramview(top = 10); - describe paramview(top = 2 + 2); +create view p2 as select number, {name:String} from system.numbers where number <= {top:UInt64}; +describe p2(top = 10); -- { serverError UNKNOWN_QUERY_PARAMETER } +describe p2(name = 'Biba', top = 2); + +create view p3 as select CAST(dummy, {t:String}); +describe p3(t = 'Int'); +describe p3(t = 'String'); + +SELECT * FROM p3(t = 'String'); + select arrayReduce('sum', (select groupArray(number) from paramview(top=10))); From 31026aa74a85ae777447b560f3c0b9446d2ba4ae Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Tue, 3 Sep 2024 15:03:24 +0200 Subject: [PATCH 09/10] Add a test --- tests/queries/0_stateless/03228_param_view_metadata_columns.sql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/queries/0_stateless/03228_param_view_metadata_columns.sql b/tests/queries/0_stateless/03228_param_view_metadata_columns.sql index 90e261c4445..98f80f26c1a 100644 --- a/tests/queries/0_stateless/03228_param_view_metadata_columns.sql +++ b/tests/queries/0_stateless/03228_param_view_metadata_columns.sql @@ -12,6 +12,8 @@ create view p3 as select CAST(dummy, {t:String}); describe p3(t = 'Int'); describe p3(t = 'String'); +describe (SELECT * FROM p3(t = 'Int64') union all SELECT * FROM p3(t = 'UInt64')); -- { serverError NO_COMMON_TYPE } + SELECT * FROM p3(t = 'String'); select arrayReduce('sum', (select groupArray(number) from paramview(top=10))); From 586182a045db025a3f07b51845d7342bfbcfb9bd Mon Sep 17 00:00:00 2001 From: Dmitry Novik Date: Thu, 5 Sep 2024 16:06:24 +0200 Subject: [PATCH 10/10] Fix clang-tidy build --- src/Interpreters/InterpreterDescribeQuery.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Interpreters/InterpreterDescribeQuery.cpp b/src/Interpreters/InterpreterDescribeQuery.cpp index 23b0f3ad54b..c5532799aef 100644 --- a/src/Interpreters/InterpreterDescribeQuery.cpp +++ b/src/Interpreters/InterpreterDescribeQuery.cpp @@ -183,7 +183,8 @@ void InterpreterDescribeQuery::fillColumnsFromTableFunction(const ASTTableExpres auto query = storage_view->getInMemoryMetadataPtr()->getSelectQuery().inner_query->clone(); NameToNameMap parameterized_view_values = analyzeFunctionParamValues(table_expression.table_function, current_context); StorageView::replaceQueryParametersIfParametrizedView(query, parameterized_view_values); - return fillColumnsFromSubqueryImpl(query, current_context); + fillColumnsFromSubqueryImpl(query, current_context); + return; } }