From f24c484f24279103ff535cbe777fb0120b3c8532 Mon Sep 17 00:00:00 2001 From: Daniil Demin Date: Wed, 20 Dec 2023 14:37:32 +0000 Subject: [PATCH 1/5] Enable SELECT from views Selecting from a view currently works for the following queries written in the view: - SELECT 1 - SELECT * FROM SomeTable - SELECT a, b FROM SomeTable (you can specify which columns you need) - SELECT * FROM FirstTable JOIN SecondTable ON ... - SELECT * FROM FirstTable UNION SELECT * FROM SecondTable - SELECT * FROM SomeOtherView - SELECT * FROM FirstView JOIN SecondView ON ... The idea of the implementation is the following: whenever we encounter TExprNode corresponding to a Right! read from a view, we change this node to a TExprNode, which corresponds to the compiled query, stored in the view. This is done on RewriteIO stage of the pipeline. The biggest challenge was to meet all the expectations of the CheckTx function applied to the rewritten query. --- ydb/core/kqp/gateway/kqp_metadata_loader.cpp | 33 +++- .../logical/kqp_opt_log_ranges_predext.cpp | 1 + ydb/core/kqp/provider/rewrite_io_utils.cpp | 169 ++++++++++++++++++ ydb/core/kqp/provider/rewrite_io_utils.h | 14 ++ ydb/core/kqp/provider/ya.make | 4 +- .../kqp/provider/yql_kikimr_datasource.cpp | 110 +++++++----- ydb/core/kqp/provider/yql_kikimr_gateway.h | 8 +- .../cases/multiple_tables/create_view.sql | 18 ++ .../input/cases/multiple_tables/drop_view.sql | 1 + .../cases/multiple_tables/etalon_query.sql | 21 +++ .../multiple_tables/select_from_view.sql | 3 + .../cases/multiple_views/create_view.sql | 9 + .../input/cases/multiple_views/drop_view.sql | 1 + .../cases/multiple_views/etalon_query.sql | 12 ++ .../cases/multiple_views/select_from_view.sql | 3 + .../input/cases/one_table/create_view.sql | 4 + .../view/input/cases/one_table/drop_view.sql | 1 + .../input/cases/one_table/etalon_query.sql | 7 + .../cases/one_table/select_from_view.sql | 3 + .../view/input/cases/one_view/create_view.sql | 4 + .../view/input/cases/one_view/drop_view.sql | 1 + .../input/cases/one_view/etalon_query.sql | 7 + .../input/cases/one_view/select_from_view.sql | 3 + .../view/input/cases/scalar/create_view.sql | 3 + .../ut/view/input/cases/scalar/drop_view.sql | 1 + .../view/input/cases/scalar/etalon_query.sql | 6 + .../input/cases/scalar/select_from_view.sql | 3 + .../input/cases/two_tables/create_view.sql | 9 + .../view/input/cases/two_tables/drop_view.sql | 1 + .../input/cases/two_tables/etalon_query.sql | 12 ++ .../cases/two_tables/select_from_view.sql | 3 + .../create_tables_and_secondary_views.sql | 40 +++++ ydb/core/kqp/ut/view/input/fill_tables.sql | 106 +++++++++++ ydb/core/kqp/ut/view/view_ut.cpp | 110 ++++++++++++ ydb/core/kqp/ut/view/ya.make | 3 + 35 files changed, 683 insertions(+), 51 deletions(-) create mode 100644 ydb/core/kqp/provider/rewrite_io_utils.cpp create mode 100644 ydb/core/kqp/provider/rewrite_io_utils.h create mode 100644 ydb/core/kqp/ut/view/input/cases/multiple_tables/create_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/multiple_tables/drop_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/multiple_tables/etalon_query.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/multiple_tables/select_from_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/multiple_views/create_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/multiple_views/drop_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/multiple_views/etalon_query.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/multiple_views/select_from_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/one_table/create_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/one_table/drop_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/one_table/etalon_query.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/one_table/select_from_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/one_view/create_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/one_view/drop_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/one_view/etalon_query.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/one_view/select_from_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/scalar/create_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/scalar/drop_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/scalar/etalon_query.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/scalar/select_from_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/two_tables/create_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/two_tables/drop_view.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/two_tables/etalon_query.sql create mode 100644 ydb/core/kqp/ut/view/input/cases/two_tables/select_from_view.sql create mode 100644 ydb/core/kqp/ut/view/input/create_tables_and_secondary_views.sql create mode 100644 ydb/core/kqp/ut/view/input/fill_tables.sql diff --git a/ydb/core/kqp/gateway/kqp_metadata_loader.cpp b/ydb/core/kqp/gateway/kqp_metadata_loader.cpp index 3f8f887e21c0..d123c2d051f8 100644 --- a/ydb/core/kqp/gateway/kqp_metadata_loader.cpp +++ b/ydb/core/kqp/gateway/kqp_metadata_loader.cpp @@ -278,6 +278,26 @@ TTableMetadataResult GetExternalDataSourceMetadataResult(const NSchemeCache::TSc return result; } +TTableMetadataResult GetViewMetadataResult(const NSchemeCache::TSchemeCacheNavigate::TEntry& schemeEntry, + const TString& cluster, + const TString& viewName) { + const auto& description = schemeEntry.ViewInfo->Description; + + TTableMetadataResult builtResult; + builtResult.SetSuccess(); + + builtResult.Metadata = new NYql::TKikimrTableMetadata(cluster, viewName); + auto metadata = builtResult.Metadata; + metadata->DoesExist = true; + metadata->PathId = NYql::TKikimrPathId(description.GetPathId().GetOwnerId(), description.GetPathId().GetLocalId()); + metadata->SchemaVersion = description.GetVersion(); + metadata->Kind = NYql::EKikimrTableKind::View; + metadata->Attributes = schemeEntry.Attributes; + metadata->ViewPersistedData = {description.GetQueryText()}; + + return builtResult; +} + TTableMetadataResult GetLoadTableMetadataResult(const NSchemeCache::TSchemeCacheNavigate::TEntry& entry, const TString& cluster, const TString& tableName, std::optional queryName = std::nullopt) { using TResult = NYql::IKikimrGateway::TTableMetadataResult; @@ -306,7 +326,11 @@ TTableMetadataResult GetLoadTableMetadataResult(const NSchemeCache::TSchemeCache return ResultFromError(ToString(entry.Status)); } - YQL_ENSURE(entry.Kind == EKind::KindTable || entry.Kind == EKind::KindColumnTable || entry.Kind == EKind::KindExternalTable || entry.Kind == EKind::KindExternalDataSource); + YQL_ENSURE(IsIn({EKind::KindTable, + EKind::KindColumnTable, + EKind::KindExternalTable, + EKind::KindExternalDataSource, + EKind::KindView}, entry.Kind)); TTableMetadataResult result; switch (entry.Kind) { @@ -316,6 +340,9 @@ TTableMetadataResult GetLoadTableMetadataResult(const NSchemeCache::TSchemeCache case EKind::KindExternalDataSource: result = GetExternalDataSourceMetadataResult(entry, cluster, tableName); break; + case EKind::KindView: + result = GetViewMetadataResult(entry, cluster, tableName); + break; default: result = GetTableMetadataResult(entry, cluster, tableName, queryName); } @@ -715,7 +742,9 @@ NThreading::TFuture TKqpTableMetadataLoader::LoadTableMeta return; } - if (!IsIn({EKind::KindExternalDataSource, EKind::KindExternalTable}, entry.Kind) && expectedSchemaVersion && entry.TableId.SchemaVersion) { + if (!IsIn({EKind::KindExternalDataSource, + EKind::KindExternalTable, + EKind::KindView}, entry.Kind) && expectedSchemaVersion && entry.TableId.SchemaVersion) { if (entry.TableId.SchemaVersion != expectedSchemaVersion) { const auto message = TStringBuilder() << "schema version mismatch during metadata loading for: " diff --git a/ydb/core/kqp/opt/logical/kqp_opt_log_ranges_predext.cpp b/ydb/core/kqp/opt/logical/kqp_opt_log_ranges_predext.cpp index 1f627bc45af3..dcad772325fb 100644 --- a/ydb/core/kqp/opt/logical/kqp_opt_log_ranges_predext.cpp +++ b/ydb/core/kqp/opt/logical/kqp_opt_log_ranges_predext.cpp @@ -55,6 +55,7 @@ TMaybeNode TryBuildTrivialReadTable(TCoFlatMap& flatmap, TKqlReadTabl break; case EKikimrTableKind::Olap: case EKikimrTableKind::External: + case EKikimrTableKind::View: case EKikimrTableKind::Unspecified: return {}; } diff --git a/ydb/core/kqp/provider/rewrite_io_utils.cpp b/ydb/core/kqp/provider/rewrite_io_utils.cpp new file mode 100644 index 000000000000..a1ba9a502251 --- /dev/null +++ b/ydb/core/kqp/provider/rewrite_io_utils.cpp @@ -0,0 +1,169 @@ +#include "rewrite_io_utils.h" + +#include +#include +#include +#include +#include +#include + +namespace NYql { +namespace { + +using namespace NNodes; + +constexpr const char* QueryGraphNodeSignature = "SavedQueryGraph"; + +NSQLTranslation::TTranslationSettings CreateViewTranslationSettings(const TString& cluster) { + NSQLTranslation::TTranslationSettings settings; + + settings.DefaultCluster = cluster; + settings.ClusterMapping[cluster] = TString(NYql::KikimrProviderName); + settings.Mode = NSQLTranslation::ESqlMode::LIMITED_VIEW; + + return settings; +} + +TExprNode::TPtr CompileQuery( + const TString& query, + TExprContext& ctx, + const TString& cluster +) { + TAstParseResult queryAst; + queryAst = NSQLTranslation::SqlToYql(query, CreateViewTranslationSettings(cluster)); + + ctx.IssueManager.AddIssues(queryAst.Issues); + if (!queryAst.IsOk()) { + return nullptr; + } + + TExprNode::TPtr queryGraph; + if (!CompileExpr(*queryAst.Root, queryGraph, ctx, nullptr, nullptr)) { + return nullptr; + } + + return queryGraph; +} + +bool ContainsNullNode(const TExprNode::TPtr& root) { + return static_cast(FindNode(root, [](const TExprNode::TPtr& node) { + return !node; + })); +} + +void AddChild(const TExprNode::TPtr& parent, const TExprNode::TPtr& newChild) { + auto childrenToChange = parent->ChildrenList(); + childrenToChange.emplace_back(newChild); + parent->ChangeChildrenInplace(std::move(childrenToChange)); +} + +void ChangeChild(const TExprNode::TPtr& parent, ui32 index, const TExprNode::TPtr& newChild) { + Y_ENSURE(parent->ChildrenSize() > index); + + auto childrenToChange = parent->ChildrenList(); + childrenToChange[index] = newChild; + parent->ChangeChildrenInplace(std::move(childrenToChange)); +} + +TExprNode::TPtr FindSavedQueryGraph(const TExprNode::TPtr& carrier) { + if (carrier->ChildrenSize() == 0) { + return nullptr; + } + auto lastChild = carrier->Children().back(); + return lastChild->IsCallable(QueryGraphNodeSignature) ? lastChild->ChildPtr(0) : TExprNode::TPtr(); +} + +void SaveQueryGraph(const TExprNode::TPtr& carrier, TExprContext& ctx, const TExprNode::TPtr& payload) { + AddChild(carrier, ctx.NewCallable(payload->Pos(), QueryGraphNodeSignature, {payload})); +} + +void InsertExecutionOrderDependencies(const TExprNode::TPtr& queryGraph, const TExprNode::TPtr& worldBefore) { + VisitExpr( + queryGraph, + nullptr, + [&worldBefore](const TExprNode::TPtr& node) { + if (node->ChildrenSize() > 0 && node->Child(0)->IsWorld()) { + ChangeChild(node, 0, worldBefore); + } + return true; + } + ); +} + +bool CheckTopLevelness(const TExprNode::TPtr& candidateRead, const TExprNode::TPtr& queryGraph) { + THashSet readsInCandidateSubgraph; + VisitExpr(candidateRead, [&readsInCandidateSubgraph](const TExprNode::TPtr& node) { + if (node->IsCallable(ReadName)) { + readsInCandidateSubgraph.emplace(node); + } + return true; + }); + + return !FindNode(queryGraph, [&readsInCandidateSubgraph](const TExprNode::TPtr& node) { + return node->IsCallable(ReadName) && !readsInCandidateSubgraph.contains(node); + }); +} + +TExprNode::TPtr FindTopLevelRead(const TExprNode::TPtr& queryGraph) { + const TExprNode::TPtr* lastReadInTopologicalOrder = nullptr; + VisitExpr( + queryGraph, + nullptr, + [&lastReadInTopologicalOrder](const TExprNode::TPtr& node) { + if (node->IsCallable(ReadName)) { + lastReadInTopologicalOrder = &node; + } + return true; + } + ); + + if (!lastReadInTopologicalOrder) { + return nullptr; + } + + Y_ENSURE(CheckTopLevelness(*lastReadInTopologicalOrder, queryGraph), + "Info for developers: assumption that there is only one top level Read! is wrong\ + for the expression graph of the query stored in the view:\n" + << queryGraph->Dump()); + + return *lastReadInTopologicalOrder; +} + +} + +TExprNode::TPtr RewriteReadFromView( + const TExprNode::TPtr& node, + TExprContext& ctx, + const TString& query, + const TString& cluster +) { + const auto readNode = node->ChildPtr(0); + const auto worldBeforeThisRead = readNode->ChildPtr(0); + + TExprNode::TPtr queryGraph = FindSavedQueryGraph(readNode); + if (!queryGraph) { + queryGraph = CompileQuery(query, ctx, cluster); + if (!queryGraph || ContainsNullNode(queryGraph)) { + ctx.AddError(TIssue(readNode->Pos(ctx), + "The query stored in the view contains errors and cannot be compiled.")); + return nullptr; + } + YQL_CLOG(TRACE, ProviderKqp) << "Expression graph of the query stored in the view:\n" + << NCommon::ExprToPrettyString(ctx, *queryGraph); + + InsertExecutionOrderDependencies(queryGraph, worldBeforeThisRead); + SaveQueryGraph(readNode, ctx, queryGraph); + } + + if (node->IsCallable(RightName)) { + return queryGraph; + } + + const auto topLevelRead = FindTopLevelRead(queryGraph); + if (!topLevelRead) { + return worldBeforeThisRead; + } + return Build(ctx, node->Pos()).Input(topLevelRead).Done().Ptr(); +} + +} \ No newline at end of file diff --git a/ydb/core/kqp/provider/rewrite_io_utils.h b/ydb/core/kqp/provider/rewrite_io_utils.h new file mode 100644 index 000000000000..02bcdfc514cf --- /dev/null +++ b/ydb/core/kqp/provider/rewrite_io_utils.h @@ -0,0 +1,14 @@ +#pragma once + +#include + +namespace NYql { + +TExprNode::TPtr RewriteReadFromView( + const TExprNode::TPtr& node, + TExprContext& ctx, + const TString& query, + const TString& cluster +); + +} \ No newline at end of file diff --git a/ydb/core/kqp/provider/ya.make b/ydb/core/kqp/provider/ya.make index 7a3b090261b5..95ced6214c37 100644 --- a/ydb/core/kqp/provider/ya.make +++ b/ydb/core/kqp/provider/ya.make @@ -1,6 +1,7 @@ LIBRARY() SRCS( + rewrite_io_utils.cpp yql_kikimr_datasink.cpp yql_kikimr_datasource.cpp yql_kikimr_exec.cpp @@ -48,9 +49,10 @@ PEERDIR( ydb/library/yql/providers/dq/expr_nodes ydb/library/yql/providers/result/expr_nodes ydb/library/yql/providers/result/provider - ydb/library/yql/sql/settings + ydb/library/yql/sql ydb/library/ydb_issue/proto ydb/library/yql/public/issue + ydb/library/yql/utils/log ) YQL_LAST_ABI_VERSION() diff --git a/ydb/core/kqp/provider/yql_kikimr_datasource.cpp b/ydb/core/kqp/provider/yql_kikimr_datasource.cpp index f9c37bd33874..e770b9afbdd1 100644 --- a/ydb/core/kqp/provider/yql_kikimr_datasource.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_datasource.cpp @@ -1,3 +1,4 @@ +#include "rewrite_io_utils.h" #include "yql_kikimr_provider_impl.h" #include @@ -673,58 +674,73 @@ class TKikimrDataSource : public TDataProviderBase { YQL_ENSURE(false, "Unsupported Kikimr KeyType."); } - auto& tableDesc = SessionCtx->Tables().GetTable(TString{source.Cluster()}, key.GetTablePath()); - if (key.GetKeyType() == TKikimrKey::Type::Table && tableDesc.Metadata->Kind == EKikimrTableKind::External && tableDesc.Metadata->ExternalSource.SourceType == ESourceType::ExternalDataSource) { - const auto& source = ExternalSourceFactory->GetOrCreate(tableDesc.Metadata->ExternalSource.Type); - ctx.Step.Repeat(TExprStep::DiscoveryIO) - .Repeat(TExprStep::Epochs) - .Repeat(TExprStep::Intents) - .Repeat(TExprStep::LoadTablesMetadata) - .Repeat(TExprStep::RewriteIO); - auto readArgs = read->ChildrenList(); - readArgs[1] = Build(ctx, node->Pos()) - .Category(ctx.NewAtom(node->Pos(), source->GetName())) - .FreeArgs() - .Add(readArgs[1]->ChildrenList()[1]) - .Build() - .Done().Ptr(); - readArgs[2] = ctx.NewCallable(node->Pos(), "MrTableConcat", { readArgs[2] }); - auto newRead = ctx.ChangeChildren(*read, std::move(readArgs)); - auto retChildren = node->ChildrenList(); - retChildren[0] = newRead; - return ctx.ChangeChildren(*node, std::move(retChildren)); - } - - if (key.GetKeyType() == TKikimrKey::Type::Table && tableDesc.Metadata->Kind == EKikimrTableKind::External && tableDesc.Metadata->ExternalSource.SourceType == ESourceType::ExternalTable) { - const auto& source = ExternalSourceFactory->GetOrCreate(tableDesc.Metadata->ExternalSource.Type); - ctx.Step.Repeat(TExprStep::DiscoveryIO) + const TString cluster = source.Cluster().StringValue(); + const TString tablePath = key.GetTablePath(); + auto& tableDesc = SessionCtx->Tables().GetTable(cluster, tablePath); + if (key.GetKeyType() == TKikimrKey::Type::Table) { + if (tableDesc.Metadata->Kind == EKikimrTableKind::External) { + if (tableDesc.Metadata->ExternalSource.SourceType == ESourceType::ExternalDataSource) { + const auto& source = ExternalSourceFactory->GetOrCreate(tableDesc.Metadata->ExternalSource.Type); + ctx.Step.Repeat(TExprStep::DiscoveryIO) + .Repeat(TExprStep::Epochs) + .Repeat(TExprStep::Intents) + .Repeat(TExprStep::LoadTablesMetadata) + .Repeat(TExprStep::RewriteIO); + auto readArgs = read->ChildrenList(); + readArgs[1] = Build(ctx, node->Pos()) + .Category(ctx.NewAtom(node->Pos(), source->GetName())) + .FreeArgs() + .Add(readArgs[1]->ChildrenList()[1]) + .Build() + .Done().Ptr(); + readArgs[2] = ctx.NewCallable(node->Pos(), "MrTableConcat", { readArgs[2] }); + auto newRead = ctx.ChangeChildren(*read, std::move(readArgs)); + auto retChildren = node->ChildrenList(); + retChildren[0] = newRead; + return ctx.ChangeChildren(*node, std::move(retChildren)); + } else if (tableDesc.Metadata->ExternalSource.SourceType == ESourceType::ExternalTable) { + const auto& source = ExternalSourceFactory->GetOrCreate(tableDesc.Metadata->ExternalSource.Type); + ctx.Step.Repeat(TExprStep::DiscoveryIO) + .Repeat(TExprStep::Epochs) + .Repeat(TExprStep::Intents) + .Repeat(TExprStep::LoadTablesMetadata) + .Repeat(TExprStep::RewriteIO); + TExprNode::TPtr path = ctx.NewCallable(node->Pos(), "String", { ctx.NewAtom(node->Pos(), tableDesc.Metadata->ExternalSource.TableLocation) }); + auto table = ctx.NewList(node->Pos(), {ctx.NewAtom(node->Pos(), "table"), path}); + auto newKey = ctx.NewCallable(node->Pos(), "Key", {table}); + auto newRead = Build(ctx, node->Pos()) + .World(read->Child(0)) + .DataSource( + Build(ctx, node->Pos()) + .Category(ctx.NewAtom(node->Pos(), source->GetName())) + .FreeArgs() + .Add(ctx.NewAtom(node->Pos(), tableDesc.Metadata->ExternalSource.DataSourcePath)) + .Build() + .Done().Ptr() + ) + .FreeArgs() + .Add(ctx.NewCallable(node->Pos(), "MrTableConcat", {newKey})) + .Add(ctx.NewCallable(node->Pos(), "Void", {})) + .Add(BuildExternalTableSettings(node->Pos(), ctx, tableDesc.Metadata->Columns, source, tableDesc.Metadata->ExternalSource.TableContent)) + + .Build() + .Done().Ptr(); + auto retChildren = node->ChildrenList(); + retChildren[0] = newRead; + return ctx.ChangeChildren(*node, std::move(retChildren)); + } + } else if (tableDesc.Metadata->Kind == EKikimrTableKind::View) { + ctx.Step + .Repeat(TExprStep::ExprEval) + .Repeat(TExprStep::DiscoveryIO) .Repeat(TExprStep::Epochs) .Repeat(TExprStep::Intents) .Repeat(TExprStep::LoadTablesMetadata) .Repeat(TExprStep::RewriteIO); - TExprNode::TPtr path = ctx.NewCallable(node->Pos(), "String", { ctx.NewAtom(node->Pos(), tableDesc.Metadata->ExternalSource.TableLocation) }); - auto table = ctx.NewList(node->Pos(), {ctx.NewAtom(node->Pos(), "table"), path}); - auto newKey = ctx.NewCallable(node->Pos(), "Key", {table}); - auto newRead = Build(ctx, node->Pos()) - .World(read->Child(0)) - .DataSource( - Build(ctx, node->Pos()) - .Category(ctx.NewAtom(node->Pos(), source->GetName())) - .FreeArgs() - .Add(ctx.NewAtom(node->Pos(), tableDesc.Metadata->ExternalSource.DataSourcePath)) - .Build() - .Done().Ptr() - ) - .FreeArgs() - .Add(ctx.NewCallable(node->Pos(), "MrTableConcat", {newKey})) - .Add(ctx.NewCallable(node->Pos(), "Void", {})) - .Add(BuildExternalTableSettings(node->Pos(), ctx, tableDesc.Metadata->Columns, source, tableDesc.Metadata->ExternalSource.TableContent)) - .Build() - .Done().Ptr(); - auto retChildren = node->ChildrenList(); - retChildren[0] = newRead; - return ctx.ChangeChildren(*node, std::move(retChildren)); + const auto& query = tableDesc.Metadata->ViewPersistedData.QueryText; + return RewriteReadFromView(node, ctx, query, cluster); + } } auto newRead = ctx.RenameNode(*read, newName); diff --git a/ydb/core/kqp/provider/yql_kikimr_gateway.h b/ydb/core/kqp/provider/yql_kikimr_gateway.h index e2dae670bfbf..99e3cdbd6bed 100644 --- a/ydb/core/kqp/provider/yql_kikimr_gateway.h +++ b/ydb/core/kqp/provider/yql_kikimr_gateway.h @@ -340,7 +340,8 @@ enum class EKikimrTableKind : ui32 { Datashard = 1, SysView = 2, Olap = 3, - External = 4 + External = 4, + View = 5, }; enum class ETableType : ui32 { @@ -389,6 +390,10 @@ enum EMetaSerializationType : ui64 { Json = 2 }; +struct TViewPersistedData { + TString QueryText; +}; + struct TKikimrTableMetadata : public TThrRefBase { bool DoesExist = false; TString Cluster; @@ -423,6 +428,7 @@ struct TKikimrTableMetadata : public TThrRefBase { TTableSettings TableSettings; TExternalSource ExternalSource; + TViewPersistedData ViewPersistedData; TKikimrTableMetadata(const TString& cluster, const TString& table) : Cluster(cluster) diff --git a/ydb/core/kqp/ut/view/input/cases/multiple_tables/create_view.sql b/ydb/core/kqp/ut/view/input/cases/multiple_tables/create_view.sql new file mode 100644 index 000000000000..bd807e00e348 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/multiple_tables/create_view.sql @@ -0,0 +1,18 @@ +CREATE VIEW `/Root/read_from_multiple_tables` WITH (security_invoker = TRUE) AS + SELECT + * + FROM `/Root/series` + AS series + JOIN ( + SELECT + seasons.title AS seasons_title, + episodes.title AS episodes_title, + seasons.series_id AS series_id + FROM `/Root/seasons` + AS seasons + JOIN `/Root/episodes` + AS episodes + ON seasons.series_id == episodes.series_id + ) + AS seasons_and_episodes + ON series.series_id == seasons_and_episodes.series_id; diff --git a/ydb/core/kqp/ut/view/input/cases/multiple_tables/drop_view.sql b/ydb/core/kqp/ut/view/input/cases/multiple_tables/drop_view.sql new file mode 100644 index 000000000000..5fa0c8b1159e --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/multiple_tables/drop_view.sql @@ -0,0 +1 @@ +DROP VIEW `/Root/read_from_multiple_tables`; diff --git a/ydb/core/kqp/ut/view/input/cases/multiple_tables/etalon_query.sql b/ydb/core/kqp/ut/view/input/cases/multiple_tables/etalon_query.sql new file mode 100644 index 000000000000..f123a6a1dc25 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/multiple_tables/etalon_query.sql @@ -0,0 +1,21 @@ +SELECT + * +FROM ( + SELECT + * + FROM `/Root/series` + AS series + JOIN ( + SELECT + seasons.title AS seasons_title, + episodes.title AS episodes_title, + seasons.series_id AS series_id + FROM `/Root/seasons` + AS seasons + JOIN `/Root/episodes` + AS episodes + ON seasons.series_id == episodes.series_id + ) + AS seasons_and_episodes + ON series.series_id == seasons_and_episodes.series_id +); diff --git a/ydb/core/kqp/ut/view/input/cases/multiple_tables/select_from_view.sql b/ydb/core/kqp/ut/view/input/cases/multiple_tables/select_from_view.sql new file mode 100644 index 000000000000..aedc8541a51a --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/multiple_tables/select_from_view.sql @@ -0,0 +1,3 @@ +SELECT + * +FROM `/Root/read_from_multiple_tables`; diff --git a/ydb/core/kqp/ut/view/input/cases/multiple_views/create_view.sql b/ydb/core/kqp/ut/view/input/cases/multiple_views/create_view.sql new file mode 100644 index 000000000000..67eb0c8e2732 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/multiple_views/create_view.sql @@ -0,0 +1,9 @@ +CREATE VIEW `/Root/read_from_multiple_views` WITH (security_invoker = TRUE) AS + SELECT + series.title AS series_title, + seasons.title AS seasons_title + FROM `/Root/view_series` + AS series + JOIN `/Root/view_seasons` + AS seasons + ON series.series_id == seasons.series_id; diff --git a/ydb/core/kqp/ut/view/input/cases/multiple_views/drop_view.sql b/ydb/core/kqp/ut/view/input/cases/multiple_views/drop_view.sql new file mode 100644 index 000000000000..41009992c2d0 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/multiple_views/drop_view.sql @@ -0,0 +1 @@ +DROP VIEW `/Root/read_from_multiple_views`; diff --git a/ydb/core/kqp/ut/view/input/cases/multiple_views/etalon_query.sql b/ydb/core/kqp/ut/view/input/cases/multiple_views/etalon_query.sql new file mode 100644 index 000000000000..c0ba9c284ea4 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/multiple_views/etalon_query.sql @@ -0,0 +1,12 @@ +SELECT + * +FROM ( + SELECT + series.title AS series_title, + seasons.title AS seasons_title + FROM `/Root/view_series` + AS series + JOIN `/Root/view_seasons` + AS seasons + ON series.series_id == seasons.series_id +); diff --git a/ydb/core/kqp/ut/view/input/cases/multiple_views/select_from_view.sql b/ydb/core/kqp/ut/view/input/cases/multiple_views/select_from_view.sql new file mode 100644 index 000000000000..eb1962a05085 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/multiple_views/select_from_view.sql @@ -0,0 +1,3 @@ +SELECT + * +FROM `/Root/read_from_multiple_views`; diff --git a/ydb/core/kqp/ut/view/input/cases/one_table/create_view.sql b/ydb/core/kqp/ut/view/input/cases/one_table/create_view.sql new file mode 100644 index 000000000000..fd7e3c904458 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/one_table/create_view.sql @@ -0,0 +1,4 @@ +CREATE VIEW `/Root/read_from_one_table` WITH (security_invoker = TRUE) AS + SELECT + * + FROM `/Root/series`; diff --git a/ydb/core/kqp/ut/view/input/cases/one_table/drop_view.sql b/ydb/core/kqp/ut/view/input/cases/one_table/drop_view.sql new file mode 100644 index 000000000000..20c2eebbbfff --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/one_table/drop_view.sql @@ -0,0 +1 @@ +DROP VIEW `/Root/read_from_one_table`; diff --git a/ydb/core/kqp/ut/view/input/cases/one_table/etalon_query.sql b/ydb/core/kqp/ut/view/input/cases/one_table/etalon_query.sql new file mode 100644 index 000000000000..46bcd9b2ea15 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/one_table/etalon_query.sql @@ -0,0 +1,7 @@ +SELECT + * +FROM ( + SELECT + * + FROM `/Root/series` +); diff --git a/ydb/core/kqp/ut/view/input/cases/one_table/select_from_view.sql b/ydb/core/kqp/ut/view/input/cases/one_table/select_from_view.sql new file mode 100644 index 000000000000..7018220ad0ad --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/one_table/select_from_view.sql @@ -0,0 +1,3 @@ +SELECT + * +FROM `/Root/read_from_one_table`; diff --git a/ydb/core/kqp/ut/view/input/cases/one_view/create_view.sql b/ydb/core/kqp/ut/view/input/cases/one_view/create_view.sql new file mode 100644 index 000000000000..6f7e2e522cb5 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/one_view/create_view.sql @@ -0,0 +1,4 @@ +CREATE VIEW `/Root/read_from_one_view` WITH (security_invoker = TRUE) AS + SELECT + * + FROM `/Root/view_series`; diff --git a/ydb/core/kqp/ut/view/input/cases/one_view/drop_view.sql b/ydb/core/kqp/ut/view/input/cases/one_view/drop_view.sql new file mode 100644 index 000000000000..32aa77b2ca7e --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/one_view/drop_view.sql @@ -0,0 +1 @@ +DROP VIEW `/Root/read_from_one_view`; diff --git a/ydb/core/kqp/ut/view/input/cases/one_view/etalon_query.sql b/ydb/core/kqp/ut/view/input/cases/one_view/etalon_query.sql new file mode 100644 index 000000000000..46bcd9b2ea15 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/one_view/etalon_query.sql @@ -0,0 +1,7 @@ +SELECT + * +FROM ( + SELECT + * + FROM `/Root/series` +); diff --git a/ydb/core/kqp/ut/view/input/cases/one_view/select_from_view.sql b/ydb/core/kqp/ut/view/input/cases/one_view/select_from_view.sql new file mode 100644 index 000000000000..0a3b65acf057 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/one_view/select_from_view.sql @@ -0,0 +1,3 @@ +SELECT + * +FROM `/Root/read_from_one_view`; diff --git a/ydb/core/kqp/ut/view/input/cases/scalar/create_view.sql b/ydb/core/kqp/ut/view/input/cases/scalar/create_view.sql new file mode 100644 index 000000000000..e436f66106f0 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/scalar/create_view.sql @@ -0,0 +1,3 @@ +CREATE VIEW `/Root/read_from_scalar` WITH (security_invoker = TRUE) AS + SELECT + 1; diff --git a/ydb/core/kqp/ut/view/input/cases/scalar/drop_view.sql b/ydb/core/kqp/ut/view/input/cases/scalar/drop_view.sql new file mode 100644 index 000000000000..8a9dce3a5fa5 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/scalar/drop_view.sql @@ -0,0 +1 @@ +DROP VIEW `/Root/read_from_scalar`; diff --git a/ydb/core/kqp/ut/view/input/cases/scalar/etalon_query.sql b/ydb/core/kqp/ut/view/input/cases/scalar/etalon_query.sql new file mode 100644 index 000000000000..809742a8d063 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/scalar/etalon_query.sql @@ -0,0 +1,6 @@ +SELECT + * +FROM ( + SELECT + 1 +); diff --git a/ydb/core/kqp/ut/view/input/cases/scalar/select_from_view.sql b/ydb/core/kqp/ut/view/input/cases/scalar/select_from_view.sql new file mode 100644 index 000000000000..f240d20c77d4 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/scalar/select_from_view.sql @@ -0,0 +1,3 @@ +SELECT + * +FROM `/Root/read_from_scalar`; diff --git a/ydb/core/kqp/ut/view/input/cases/two_tables/create_view.sql b/ydb/core/kqp/ut/view/input/cases/two_tables/create_view.sql new file mode 100644 index 000000000000..cea657894d94 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/two_tables/create_view.sql @@ -0,0 +1,9 @@ +CREATE VIEW `/Root/read_from_two_tables` WITH (security_invoker = TRUE) AS + SELECT + series.title AS series_title, + seasons.title AS seasons_title + FROM `/Root/series` + AS series + JOIN `/Root/seasons` + AS seasons + ON series.series_id == seasons.series_id; diff --git a/ydb/core/kqp/ut/view/input/cases/two_tables/drop_view.sql b/ydb/core/kqp/ut/view/input/cases/two_tables/drop_view.sql new file mode 100644 index 000000000000..7e3615f51f60 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/two_tables/drop_view.sql @@ -0,0 +1 @@ +DROP VIEW `/Root/read_from_two_tables`; diff --git a/ydb/core/kqp/ut/view/input/cases/two_tables/etalon_query.sql b/ydb/core/kqp/ut/view/input/cases/two_tables/etalon_query.sql new file mode 100644 index 000000000000..f6be743ed1fc --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/two_tables/etalon_query.sql @@ -0,0 +1,12 @@ +SELECT + * +FROM ( + SELECT + series.title AS series_title, + seasons.title AS seasons_title + FROM `/Root/series` + AS series + JOIN `/Root/seasons` + AS seasons + ON series.series_id == seasons.series_id +); diff --git a/ydb/core/kqp/ut/view/input/cases/two_tables/select_from_view.sql b/ydb/core/kqp/ut/view/input/cases/two_tables/select_from_view.sql new file mode 100644 index 000000000000..922bc55ebe31 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/cases/two_tables/select_from_view.sql @@ -0,0 +1,3 @@ +SELECT + * +FROM `/Root/read_from_two_tables`; diff --git a/ydb/core/kqp/ut/view/input/create_tables_and_secondary_views.sql b/ydb/core/kqp/ut/view/input/create_tables_and_secondary_views.sql new file mode 100644 index 000000000000..21041764ed40 --- /dev/null +++ b/ydb/core/kqp/ut/view/input/create_tables_and_secondary_views.sql @@ -0,0 +1,40 @@ +CREATE TABLE `/Root/series` ( + series_id Uint64, + title Utf8, + series_info Utf8, + release_date Date, + PRIMARY KEY (series_id) +); + +CREATE TABLE `/Root/seasons` ( + series_id Uint64, + season_id Uint64, + title Utf8, + first_aired Date, + last_aired Date, + PRIMARY KEY (series_id, season_id) +); + +CREATE TABLE `/Root/episodes` ( + series_id Uint64, + season_id Uint64, + episode_id Uint64, + title Utf8, + air_date Date, + PRIMARY KEY (series_id, season_id, episode_id) +); + +CREATE VIEW `/Root/view_series` WITH (security_invoker = TRUE) AS + SELECT + * + FROM `/Root/series`; + +CREATE VIEW `/Root/view_seasons` WITH (security_invoker = TRUE) AS + SELECT + * + FROM `/Root/seasons`; + +CREATE VIEW `/Root/view_episodes` WITH (security_invoker = TRUE) AS + SELECT + * + FROM `/Root/episodes`; diff --git a/ydb/core/kqp/ut/view/input/fill_tables.sql b/ydb/core/kqp/ut/view/input/fill_tables.sql new file mode 100644 index 000000000000..6a80accf420b --- /dev/null +++ b/ydb/core/kqp/ut/view/input/fill_tables.sql @@ -0,0 +1,106 @@ +REPLACE INTO `/Root/series` ( + series_id, + title, + release_date, + series_info +) +VALUES + (1, "IT Crowd", Date("2006-02-03"), "The IT Crowd is a British sitcom produced by Channel 4, written by Graham Linehan, produced by Ash Atalla and starring Chris O'Dowd, Richard Ayoade, Katherine Parkinson, and Matt Berry."), + (2, "Silicon Valley", Date("2014-04-06"), "Silicon Valley is an American comedy television series created by Mike Judge, John Altschuler and Dave Krinsky. The series focuses on five young men who founded a startup company in Silicon Valley."); + +REPLACE INTO `/Root/seasons` ( + series_id, + season_id, + title, + first_aired, + last_aired +) +VALUES + (1, 1, "Season 1", Date("2006-02-03"), Date("2006-03-03")), + (1, 2, "Season 2", Date("2007-08-24"), Date("2007-09-28")), + (1, 3, "Season 3", Date("2008-11-21"), Date("2008-12-26")), + (1, 4, "Season 4", Date("2010-06-25"), Date("2010-07-30")), + (2, 1, "Season 1", Date("2014-04-06"), Date("2014-06-01")), + (2, 2, "Season 2", Date("2015-04-12"), Date("2015-06-14")), + (2, 3, "Season 3", Date("2016-04-24"), Date("2016-06-26")), + (2, 4, "Season 4", Date("2017-04-23"), Date("2017-06-25")), + (2, 5, "Season 5", Date("2018-03-25"), Date("2018-05-13")); + +REPLACE INTO `/Root/episodes` ( + series_id, + season_id, + episode_id, + title, + air_date +) +VALUES + (1, 1, 1, "Yesterday's Jam", Date("2006-02-03")), + (1, 1, 2, "Calamity Jen", Date("2006-02-03")), + (1, 1, 3, "Fifty-Fifty", Date("2006-02-10")), + (1, 1, 4, "The Red Door", Date("2006-02-17")), + (1, 1, 5, "The Haunting of Bill Crouse", Date("2006-02-24")), + (1, 1, 6, "Aunt Irma Visits", Date("2006-03-03")), + (1, 2, 1, "The Work Outing", Date("2006-08-24")), + (1, 2, 2, "Return of the Golden Child", Date("2007-08-31")), + (1, 2, 3, "Moss and the German", Date("2007-09-07")), + (1, 2, 4, "The Dinner Party", Date("2007-09-14")), + (1, 2, 5, "Smoke and Mirrors", Date("2007-09-21")), + (1, 2, 6, "Men Without Women", Date("2007-09-28")), + (1, 3, 1, "From Hell", Date("2008-11-21")), + (1, 3, 2, "Are We Not Men?", Date("2008-11-28")), + (1, 3, 3, "Tramps Like Us", Date("2008-12-05")), + (1, 3, 4, "The Speech", Date("2008-12-12")), + (1, 3, 5, "Friendface", Date("2008-12-19")), + (1, 3, 6, "Calendar Geeks", Date("2008-12-26")), + (1, 4, 1, "Jen The Fredo", Date("2010-06-25")), + (1, 4, 2, "The Final Countdown", Date("2010-07-02")), + (1, 4, 3, "Something Happened", Date("2010-07-09")), + (1, 4, 4, "Italian For Beginners", Date("2010-07-16")), + (1, 4, 5, "Bad Boys", Date("2010-07-23")), + (1, 4, 6, "Reynholm vs Reynholm", Date("2010-07-30")), + (2, 1, 1, "Minimum Viable Product", Date("2014-04-06")), + (2, 1, 2, "The Cap Table", Date("2014-04-13")), + (2, 1, 3, "Articles of Incorporation", Date("2014-04-20")), + (2, 1, 4, "Fiduciary Duties", Date("2014-04-27")), + (2, 1, 5, "Signaling Risk", Date("2014-05-04")), + (2, 1, 6, "Third Party Insourcing", Date("2014-05-11")), + (2, 1, 7, "Proof of Concept", Date("2014-05-18")), + (2, 1, 8, "Optimal Tip-to-Tip Efficiency", Date("2014-06-01")), + (2, 2, 1, "Sand Hill Shuffle", Date("2015-04-12")), + (2, 2, 2, "Runaway Devaluation", Date("2015-04-19")), + (2, 2, 3, "Bad Money", Date("2015-04-26")), + (2, 2, 4, "The Lady", Date("2015-05-03")), + (2, 2, 5, "Server Space", Date("2015-05-10")), + (2, 2, 6, "Homicide", Date("2015-05-17")), + (2, 2, 7, "Adult Content", Date("2015-05-24")), + (2, 2, 8, "White Hat/Black Hat", Date("2015-05-31")), + (2, 2, 9, "Binding Arbitration", Date("2015-06-07")), + (2, 2, 10, "Two Days of the Condor", Date("2015-06-14")), + (2, 3, 1, "Founder Friendly", Date("2016-04-24")), + (2, 3, 2, "Two in the Box", Date("2016-05-01")), + (2, 3, 3, "Meinertzhagen's Haversack", Date("2016-05-08")), + (2, 3, 4, "Maleant Data Systems Solutions", Date("2016-05-15")), + (2, 3, 5, "The Empty Chair", Date("2016-05-22")), + (2, 3, 6, "Bachmanity Insanity", Date("2016-05-29")), + (2, 3, 7, "To Build a Better Beta", Date("2016-06-05")), + (2, 3, 8, "Bachman's Earnings Over-Ride", Date("2016-06-12")), + (2, 3, 9, "Daily Active Users", Date("2016-06-19")), + (2, 3, 10, "The Uptick", Date("2016-06-26")), + (2, 4, 1, "Success Failure", Date("2017-04-23")), + (2, 4, 2, "Terms of Service", Date("2017-04-30")), + (2, 4, 3, "Intellectual Property", Date("2017-05-07")), + (2, 4, 4, "Teambuilding Exercise", Date("2017-05-14")), + (2, 4, 5, "The Blood Boy", Date("2017-05-21")), + (2, 4, 6, "Customer Service", Date("2017-05-28")), + (2, 4, 7, "The Patent Troll", Date("2017-06-04")), + (2, 4, 8, "The Keenan Vortex", Date("2017-06-11")), + (2, 4, 9, "Hooli-Con", Date("2017-06-18")), + (2, 4, 10, "Server Error", Date("2017-06-25")), + (2, 5, 1, "Grow Fast or Die Slow", Date("2018-03-25")), + (2, 5, 2, "Reorientation", Date("2018-04-01")), + (2, 5, 3, "Chief Operating Officer", Date("2018-04-08")), + (2, 5, 4, "Tech Evangelist", Date("2018-04-15")), + (2, 5, 5, "Facial Recognition", Date("2018-04-22")), + (2, 5, 6, "Artificial Emotional Intelligence", Date("2018-04-29")), + (2, 5, 7, "Initial Coin Offering", Date("2018-05-06")), + (2, 5, 8, "Fifty-One Percent", Date("2018-05-13")); diff --git a/ydb/core/kqp/ut/view/view_ut.cpp b/ydb/core/kqp/ut/view/view_ut.cpp index d6326bcb93d9..1988ce52ae02 100644 --- a/ydb/core/kqp/ut/view/view_ut.cpp +++ b/ydb/core/kqp/ut/view/view_ut.cpp @@ -1,5 +1,8 @@ #include #include +#include + +#include #include @@ -34,11 +37,55 @@ void ExpectUnknownEntry(TTestActorRuntime& runtime, const TString& path) { UNIT_ASSERT_EQUAL(pathEntry.Kind, NSchemeCache::TSchemeCacheNavigate::EKind::KindUnknown); } +void EnableLogging() { + using namespace NYql::NLog; + YqlLogger().ResetBackend(CreateLogBackend("cerr")); + for (const auto component : {EComponent::Default, EComponent::Sql, EComponent::ProviderKqp}) { + YqlLogger().SetComponentLevel(component, ELevel::INFO); + } +} + +TString ReadWholeFile(const TString& path) { + TFileInput file(path); + return file.ReadAll(); +} + void ExecuteDataDefinitionQuery(TSession& session, const TString& script) { + Cerr << "Executing the following DDL script:\n" << script; + const auto result = session.ExecuteSchemeQuery(script, {}).ExtractValueSync(); UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); } +TDataQueryResult ExecuteDataModificationQuery(TSession& session, const TString& script) { + Cerr << "Executing the following DML script:\n" << script; + + const auto result = session.ExecuteDataQuery( + script, + TTxControl::BeginTx(TTxSettings::SerializableRW()).CommitTx(), + {} + ).ExtractValueSync(); + UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); + + return result; +} + +void CompareResults(const TDataQueryResult& first, const TDataQueryResult& second) { + const auto& firstResults = first.GetResultSets(); + const auto& secondResults = second.GetResultSets(); + + UNIT_ASSERT_VALUES_EQUAL(firstResults.size(), secondResults.size()); + for (size_t i = 0; i < firstResults.size(); ++i) { + CompareYson(FormatResultSetYson(firstResults[i]), FormatResultSetYson(secondResults[i])); + } +} + +void InitializeTablesAndSecondaryViews(TSession& session) { + const auto inputFolder = ArcadiaFromCurrentLocation(__SOURCE_FILE__, "input"); + ExecuteDataDefinitionQuery(session, ReadWholeFile(inputFolder + "/create_tables_and_secondary_views.sql")); + ExecuteDataModificationQuery(session, ReadWholeFile(inputFolder + "/fill_tables.sql")); +} + } Y_UNIT_TEST_SUITE(TKQPViewTest) { @@ -195,3 +242,66 @@ Y_UNIT_TEST_SUITE(TKQPViewTest) { } } } + +Y_UNIT_TEST_SUITE(TSelectFromViewTest) { + + Y_UNIT_TEST(OneTable) { + TKikimrRunner kikimr; + SetEnableViewsFeatureFlag(kikimr); + auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); + + constexpr const char* viewName = "/Root/TheView"; + constexpr const char* testTable = "/Root/Test"; + const auto innerQuery = std::format(R"( + SELECT * FROM `{}` + )", + testTable + ); + + const TString creationQuery = std::format(R"( + CREATE VIEW `{}` WITH (security_invoker = true) AS {}; + )", + viewName, + innerQuery + ); + ExecuteDataDefinitionQuery(session, creationQuery); + + const auto etalonResults = ExecuteDataModificationQuery(session, std::format(R"( + SELECT * FROM ({}); + )", + innerQuery + ) + ); + const auto selectFromViewResults = ExecuteDataModificationQuery(session, std::format(R"( + SELECT * FROM `{}`; + )", + viewName + ) + ); + CompareResults(etalonResults, selectFromViewResults); + } + + Y_UNIT_TEST(ReadTestCasesFromFiles) { + TKikimrRunner kikimr; + SetEnableViewsFeatureFlag(kikimr); + auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); + + InitializeTablesAndSecondaryViews(session); + EnableLogging(); + + const auto testcasesFolder = ArcadiaFromCurrentLocation(__SOURCE_FILE__, "input/cases"); + TDirsList testcases; + testcases.Fill(testcasesFolder); + TString testcase; + while (testcase = testcases.Next()) { + const auto pathPrefix = TStringBuilder() << testcasesFolder << '/' << testcase << '/'; + ExecuteDataDefinitionQuery(session, ReadWholeFile(pathPrefix + "create_view.sql")); + + const auto etalonResults = ExecuteDataModificationQuery(session, ReadWholeFile(pathPrefix + "etalon_query.sql")); + const auto selectFromViewResults = ExecuteDataModificationQuery(session, ReadWholeFile(pathPrefix + "select_from_view.sql")); + CompareResults(etalonResults, selectFromViewResults); + + ExecuteDataDefinitionQuery(session, ReadWholeFile(pathPrefix + "drop_view.sql")); + } + } +} diff --git a/ydb/core/kqp/ut/view/ya.make b/ydb/core/kqp/ut/view/ya.make index 3e3f7f8d8539..6dc45fbd764c 100644 --- a/ydb/core/kqp/ut/view/ya.make +++ b/ydb/core/kqp/ut/view/ya.make @@ -11,10 +11,13 @@ SRCS( PEERDIR( ydb/core/kqp/ut/common ydb/library/yql/sql + ydb/library/yql/utils/log ydb/core/testlib/basics/default ) +DATA(arcadia/ydb/core/kqp/ut/view/input) + YQL_LAST_ABI_VERSION() END() From a5c0949d554d7f390f14e8139785af961411e87e Mon Sep 17 00:00:00 2001 From: Daniil Demin Date: Thu, 28 Dec 2023 14:29:09 +0000 Subject: [PATCH 2/5] Add unit tests for the absence of an explicit WITH (security_invoker = TRUE) option. Add TablePrefixPath pragma test for CREATE VIEW / DROP VIEW commands --- ydb/library/yql/sql/v1/sql_ut.cpp | 68 +++++++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/ydb/library/yql/sql/v1/sql_ut.cpp b/ydb/library/yql/sql/v1/sql_ut.cpp index 6e2a13ecfb1d..5171e5188140 100644 --- a/ydb/library/yql/sql/v1/sql_ut.cpp +++ b/ydb/library/yql/sql/v1/sql_ut.cpp @@ -6134,12 +6134,30 @@ Y_UNIT_TEST_SUITE(TViewSyntaxTest) { Y_UNIT_TEST(CreateViewSimple) { NYql::TAstParseResult res = SqlToYql(R"( USE plato; - CREATE VIEW TheView WITH (security_invoker = true) AS SELECT 1; + CREATE VIEW TheView WITH (security_invoker = TRUE) AS SELECT 1; )" ); UNIT_ASSERT_C(res.Root, res.Issues.ToString()); } + Y_UNIT_TEST(CreateViewNoSecurityInvoker) { + NYql::TAstParseResult res = SqlToYql(R"( + USE plato; + CREATE VIEW TheView AS SELECT 1; + )" + ); + UNIT_ASSERT_STRING_CONTAINS(res.Issues.ToString(), "Unexpected token 'AS' : syntax error"); + } + + Y_UNIT_TEST(CreateViewSecurityInvokerTurnedOff) { + NYql::TAstParseResult res = SqlToYql(R"( + USE plato; + CREATE VIEW TheView WITH (security_invoker = FALSE) AS SELECT 1; + )" + ); + UNIT_ASSERT_STRING_CONTAINS(res.Issues.ToString(), "SECURITY_INVOKER option must be explicitly enabled"); + } + Y_UNIT_TEST(CreateViewFromTable) { constexpr const char* path = "/PathPrefix/TheView"; constexpr const char* query = R"( @@ -6148,7 +6166,7 @@ Y_UNIT_TEST_SUITE(TViewSyntaxTest) { NYql::TAstParseResult res = SqlToYql(std::format(R"( USE plato; - CREATE VIEW `{}` WITH (security_invoker = true) AS {}; + CREATE VIEW `{}` WITH (security_invoker = TRUE) AS {}; )", path, query @@ -6176,7 +6194,7 @@ Y_UNIT_TEST_SUITE(TViewSyntaxTest) { NYql::TAstParseResult res = SqlToYql(std::format(R"( USE plato; - CREATE VIEW `{}` WITH (security_invoker = true) AS {}; + CREATE VIEW `{}` WITH (security_invoker = TRUE) AS {}; )", path, query @@ -6218,4 +6236,48 @@ Y_UNIT_TEST_SUITE(TViewSyntaxTest) { UNIT_ASSERT_VALUES_EQUAL(elementStat["Write!"], 1); } + + Y_UNIT_TEST(CreateViewWithTablePrefix) { + NYql::TAstParseResult res = SqlToYql(R"( + USE plato; + PRAGMA TablePathPrefix='/PathPrefix'; + CREATE VIEW TheView WITH (security_invoker = TRUE) AS SELECT 1; + )" + ); + UNIT_ASSERT_C(res.Root, res.Issues.ToString()); + + TVerifyLineFunc verifyLine = [](const TString& word, const TString& line) { + if (word == "Write!") { + UNIT_ASSERT_STRING_CONTAINS(line, "/PathPrefix/TheView"); + UNIT_ASSERT_STRING_CONTAINS(line, "createObject"); + } + }; + + TWordCountHive elementStat = { {"Write!"} }; + VerifyProgram(res, elementStat, verifyLine); + + UNIT_ASSERT_VALUES_EQUAL(elementStat["Write!"], 1); + } + + Y_UNIT_TEST(DropViewWithTablePrefix) { + NYql::TAstParseResult res = SqlToYql(R"( + USE plato; + PRAGMA TablePathPrefix='/PathPrefix'; + DROP VIEW TheView; + )" + ); + UNIT_ASSERT_C(res.Root, res.Issues.ToString()); + + TVerifyLineFunc verifyLine = [](const TString& word, const TString& line) { + if (word == "Write") { + UNIT_ASSERT_STRING_CONTAINS(line, "/PathPrefix/TheView"); + UNIT_ASSERT_STRING_CONTAINS(line, "dropObject"); + } + }; + + TWordCountHive elementStat = { {"Write!"} }; + VerifyProgram(res, elementStat, verifyLine); + + UNIT_ASSERT_VALUES_EQUAL(elementStat["Write!"], 1); + } } From 1a3c7bb13b61c721679387b15ab5d6d26205b911 Mon Sep 17 00:00:00 2001 From: Daniil Demin Date: Tue, 9 Jan 2024 16:20:01 +0000 Subject: [PATCH 3/5] Check the feature flag before rewriting a read from a view. Unify the error message in case of the disabled "EnableViews" feature flag with CREATE VIEW command. Slightly better formatting of error messages --- .../kqp/gateway/behaviour/view/manager.cpp | 71 ++++++++++--------- ydb/core/kqp/provider/rewrite_io_utils.cpp | 10 +-- .../kqp/provider/yql_kikimr_datasource.cpp | 6 ++ 3 files changed, 50 insertions(+), 37 deletions(-) diff --git a/ydb/core/kqp/gateway/behaviour/view/manager.cpp b/ydb/core/kqp/gateway/behaviour/view/manager.cpp index bd3cee0133d1..cfe2a73ac7d5 100644 --- a/ydb/core/kqp/gateway/behaviour/view/manager.cpp +++ b/ydb/core/kqp/gateway/behaviour/view/manager.cpp @@ -16,27 +16,31 @@ TString GetByKeyOrDefault(const NYql::TCreateObjectSettings& container, const TS return value ? *value : TString{}; } -void CheckFeatureFlag(TInternalModificationContext& context) { +TYqlConclusionStatus CheckFeatureFlag(TInternalModificationContext& context) { auto* const actorSystem = context.GetExternalData().GetActorSystem(); if (!actorSystem) { - ythrow TSystemError() << "This place needs an actor system. Please contact internal support"; + ythrow yexception() << "This place needs an actor system. Please contact internal support"; } - if (!AppData(actorSystem)->FeatureFlags.GetEnableViews()) { - ythrow TSystemError() << "Views are disabled. Please contact your system administrator to enable it"; + return AppData(actorSystem)->FeatureFlags.GetEnableViews() + ? TYqlConclusionStatus::Success() + : TYqlConclusionStatus::Fail("Views are disabled. Please contact your system administrator to enable the feature"); +} + +std::pair SplitPathByDb(const TString& objectId, + const TString& database) { + std::pair pathPair; + TString error; + if (!TrySplitPathByDb(objectId, database, pathPair, error)) { + ythrow TBadArgumentException() << error; } + return pathPair; } void FillCreateViewProposal(NKikimrSchemeOp::TModifyScheme& modifyScheme, - const NYql::TCreateObjectSettings& settings, - TInternalModificationContext& context) { + const NYql::TCreateObjectSettings& settings, + const TString& database) { - std::pair pathPair; - { - TString error; - if (!TrySplitPathByDb(settings.GetObjectId(), context.GetExternalData().GetDatabase(), pathPair, error)) { - ythrow TBadArgumentException() << error; - } - } + const auto pathPair = SplitPathByDb(settings.GetObjectId(), database); modifyScheme.SetWorkingDir(pathPair.first); modifyScheme.SetOperationType(NKikimrSchemeOp::ESchemeOpCreateView); @@ -51,15 +55,9 @@ void FillCreateViewProposal(NKikimrSchemeOp::TModifyScheme& modifyScheme, void FillDropViewProposal(NKikimrSchemeOp::TModifyScheme& modifyScheme, const NYql::TDropObjectSettings& settings, - TInternalModificationContext& context) { + const TString& database) { - std::pair pathPair; - { - TString error; - if (!TrySplitPathByDb(settings.GetObjectId(), context.GetExternalData().GetDatabase(), pathPair, error)) { - ythrow TBadArgumentException() << error; - } - } + const auto pathPair = SplitPathByDb(settings.GetObjectId(), database); modifyScheme.SetWorkingDir(pathPair.first); modifyScheme.SetOperationType(NKikimrSchemeOp::ESchemeOpDropView); @@ -92,7 +90,7 @@ NThreading::TFuture CreateView(const NYql::TCreateObjectSe proposal->Record.SetUserToken(context.GetExternalData().GetUserToken()->GetSerializedToken()); } auto& schemeTx = *proposal->Record.MutableTransaction()->MutableModifyScheme(); - FillCreateViewProposal(schemeTx, settings, context); + FillCreateViewProposal(schemeTx, settings, context.GetExternalData().GetDatabase()); return SendSchemeRequest(proposal.Release(), context.GetExternalData().GetActorSystem(), true); } @@ -105,7 +103,7 @@ NThreading::TFuture DropView(const NYql::TDropObjectSettin proposal->Record.SetUserToken(context.GetExternalData().GetUserToken()->GetSerializedToken()); } auto& schemeTx = *proposal->Record.MutableTransaction()->MutableModifyScheme(); - FillDropViewProposal(schemeTx, settings, context); + FillDropViewProposal(schemeTx, settings, context.GetExternalData().GetDatabase()); return SendSchemeRequest(proposal.Release(), context.GetExternalData().GetActorSystem(), false); } @@ -113,13 +111,13 @@ NThreading::TFuture DropView(const NYql::TDropObjectSettin void PrepareCreateView(NKqpProto::TKqpSchemeOperation& schemeOperation, const NYql::TObjectSettingsImpl& settings, TInternalModificationContext& context) { - FillCreateViewProposal(*schemeOperation.MutableCreateView(), settings, context); + FillCreateViewProposal(*schemeOperation.MutableCreateView(), settings, context.GetExternalData().GetDatabase()); } void PrepareDropView(NKqpProto::TKqpSchemeOperation& schemeOperation, const NYql::TObjectSettingsImpl& settings, TInternalModificationContext& context) { - FillDropViewProposal(*schemeOperation.MutableDropView(), settings, context); + FillDropViewProposal(*schemeOperation.MutableDropView(), settings, context.GetExternalData().GetDatabase()); } } @@ -129,21 +127,28 @@ NThreading::TFuture TViewManager::DoModify(const NYql::TOb const NMetadata::IClassBehaviour::TPtr& manager, TInternalModificationContext& context) const { Y_UNUSED(nodeId, manager); + const auto makeFuture = [](const TYqlConclusionStatus& status) { + return NThreading::MakeFuture(status); + }; try { - CheckFeatureFlag(context); + if (const auto status = CheckFeatureFlag(context); status.IsFail()) { + return makeFuture(status); + } switch (context.GetActivityType()) { case EActivityType::Alter: + return makeFuture(TYqlConclusionStatus::Fail("Alter operation for VIEW objects is not implemented")); case EActivityType::Upsert: + return makeFuture(TYqlConclusionStatus::Fail("Upsert operation for VIEW objects is not implemented")); case EActivityType::Undefined: - ythrow TBadArgumentException() << "not implemented"; + return makeFuture(TYqlConclusionStatus::Fail("Undefined operation for a VIEW object")); case EActivityType::Create: return CreateView(settings, context); case EActivityType::Drop: return DropView(settings, context); } } catch (...) { - return NThreading::MakeFuture(TYqlConclusionStatus::Fail(CurrentExceptionMessage())); + return makeFuture(TYqlConclusionStatus::Fail(CurrentExceptionMessage())); } } @@ -154,14 +159,16 @@ TViewManager::TYqlConclusionStatus TViewManager::DoPrepare(NKqpProto::TKqpScheme Y_UNUSED(manager); try { - CheckFeatureFlag(context); + if (const auto status = CheckFeatureFlag(context); status.IsFail()) { + return status; + } switch (context.GetActivityType()) { case EActivityType::Undefined: - ythrow TBadArgumentException() << "Undefined operation for a VIEW object"; + return TYqlConclusionStatus::Fail("Undefined operation for a VIEW object"); case EActivityType::Upsert: - ythrow TBadArgumentException() << "Upsert operation for VIEW objects is not implemented"; + return TYqlConclusionStatus::Fail("Upsert operation for VIEW objects is not implemented"); case EActivityType::Alter: - ythrow TBadArgumentException() << "Alter operation for VIEW objects is not implemented"; + return TYqlConclusionStatus::Fail("Alter operation for VIEW objects is not implemented"); case EActivityType::Create: PrepareCreateView(schemeOperation, settings, context); break; diff --git a/ydb/core/kqp/provider/rewrite_io_utils.cpp b/ydb/core/kqp/provider/rewrite_io_utils.cpp index a1ba9a502251..ed720abe8e17 100644 --- a/ydb/core/kqp/provider/rewrite_io_utils.cpp +++ b/ydb/core/kqp/provider/rewrite_io_utils.cpp @@ -58,7 +58,7 @@ void AddChild(const TExprNode::TPtr& parent, const TExprNode::TPtr& newChild) { } void ChangeChild(const TExprNode::TPtr& parent, ui32 index, const TExprNode::TPtr& newChild) { - Y_ENSURE(parent->ChildrenSize() > index); + YQL_ENSURE(parent->ChildrenSize() > index); auto childrenToChange = parent->ChildrenList(); childrenToChange[index] = newChild; @@ -121,10 +121,10 @@ TExprNode::TPtr FindTopLevelRead(const TExprNode::TPtr& queryGraph) { return nullptr; } - Y_ENSURE(CheckTopLevelness(*lastReadInTopologicalOrder, queryGraph), - "Info for developers: assumption that there is only one top level Read! is wrong\ - for the expression graph of the query stored in the view:\n" - << queryGraph->Dump()); + YQL_ENSURE(CheckTopLevelness(*lastReadInTopologicalOrder, queryGraph), + "Info for developers: assumption that there is only one top level Read! is wrong " + "for the expression graph of the query stored in the view:\n" + << queryGraph->Dump()); return *lastReadInTopologicalOrder; } diff --git a/ydb/core/kqp/provider/yql_kikimr_datasource.cpp b/ydb/core/kqp/provider/yql_kikimr_datasource.cpp index e770b9afbdd1..810057fe28e3 100644 --- a/ydb/core/kqp/provider/yql_kikimr_datasource.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_datasource.cpp @@ -730,6 +730,12 @@ class TKikimrDataSource : public TDataProviderBase { return ctx.ChangeChildren(*node, std::move(retChildren)); } } else if (tableDesc.Metadata->Kind == EKikimrTableKind::View) { + if (!SessionCtx->Config().FeatureFlags.GetEnableViews()) { + ctx.AddError(TIssue(node->Pos(ctx), + "Views are disabled. Please contact your system administrator to enable the feature")); + return nullptr; + } + ctx.Step .Repeat(TExprStep::ExprEval) .Repeat(TExprStep::DiscoveryIO) From 03b2738689b3df065b821e3f80a0e69a27951ea5 Mon Sep 17 00:00:00 2001 From: Daniil Demin Date: Mon, 15 Jan 2024 13:45:37 +0000 Subject: [PATCH 4/5] Review fixes - Unit tests for disabled feature flag. - Explicit error in case views haven't been rewritten at later optimizer stage. - Explicit node typing in RewriteReadFromView. - Remove unnecessary check in RewriteReadFromView. - Formatting. --- ydb/core/kqp/gateway/kqp_metadata_loader.cpp | 39 +++---- .../logical/kqp_opt_log_ranges_predext.cpp | 3 +- ydb/core/kqp/provider/rewrite_io_utils.cpp | 25 ++--- ydb/core/kqp/ut/view/view_ut.cpp | 100 +++++++++++++++--- 4 files changed, 120 insertions(+), 47 deletions(-) diff --git a/ydb/core/kqp/gateway/kqp_metadata_loader.cpp b/ydb/core/kqp/gateway/kqp_metadata_loader.cpp index d123c2d051f8..c8a7f7038cde 100644 --- a/ydb/core/kqp/gateway/kqp_metadata_loader.cpp +++ b/ydb/core/kqp/gateway/kqp_metadata_loader.cpp @@ -278,24 +278,27 @@ TTableMetadataResult GetExternalDataSourceMetadataResult(const NSchemeCache::TSc return result; } -TTableMetadataResult GetViewMetadataResult(const NSchemeCache::TSchemeCacheNavigate::TEntry& schemeEntry, - const TString& cluster, - const TString& viewName) { - const auto& description = schemeEntry.ViewInfo->Description; - - TTableMetadataResult builtResult; - builtResult.SetSuccess(); - - builtResult.Metadata = new NYql::TKikimrTableMetadata(cluster, viewName); - auto metadata = builtResult.Metadata; - metadata->DoesExist = true; - metadata->PathId = NYql::TKikimrPathId(description.GetPathId().GetOwnerId(), description.GetPathId().GetLocalId()); - metadata->SchemaVersion = description.GetVersion(); - metadata->Kind = NYql::EKikimrTableKind::View; - metadata->Attributes = schemeEntry.Attributes; - metadata->ViewPersistedData = {description.GetQueryText()}; - - return builtResult; +TTableMetadataResult GetViewMetadataResult( + const NSchemeCache::TSchemeCacheNavigate::TEntry& schemeEntry, + const TString& cluster, + const TString& viewName +) { + const auto& description = schemeEntry.ViewInfo->Description; + + TTableMetadataResult builtResult; + builtResult.SetSuccess(); + + builtResult.Metadata = new NYql::TKikimrTableMetadata(cluster, viewName); + auto metadata = builtResult.Metadata; + metadata->DoesExist = true; + metadata->PathId = NYql::TKikimrPathId(description.GetPathId().GetOwnerId(), + description.GetPathId().GetLocalId()); + metadata->SchemaVersion = description.GetVersion(); + metadata->Kind = NYql::EKikimrTableKind::View; + metadata->Attributes = schemeEntry.Attributes; + metadata->ViewPersistedData = {description.GetQueryText()}; + + return builtResult; } TTableMetadataResult GetLoadTableMetadataResult(const NSchemeCache::TSchemeCacheNavigate::TEntry& entry, diff --git a/ydb/core/kqp/opt/logical/kqp_opt_log_ranges_predext.cpp b/ydb/core/kqp/opt/logical/kqp_opt_log_ranges_predext.cpp index dcad772325fb..b3aeaed19783 100644 --- a/ydb/core/kqp/opt/logical/kqp_opt_log_ranges_predext.cpp +++ b/ydb/core/kqp/opt/logical/kqp_opt_log_ranges_predext.cpp @@ -55,9 +55,10 @@ TMaybeNode TryBuildTrivialReadTable(TCoFlatMap& flatmap, TKqlReadTabl break; case EKikimrTableKind::Olap: case EKikimrTableKind::External: - case EKikimrTableKind::View: case EKikimrTableKind::Unspecified: return {}; + case EKikimrTableKind::View: + YQL_ENSURE(false, "All views should have been rewritten at this stage."); } auto row = flatmap.Lambda().Args().Arg(0); diff --git a/ydb/core/kqp/provider/rewrite_io_utils.cpp b/ydb/core/kqp/provider/rewrite_io_utils.cpp index ed720abe8e17..49d0c4a0d6f5 100644 --- a/ydb/core/kqp/provider/rewrite_io_utils.cpp +++ b/ydb/core/kqp/provider/rewrite_io_utils.cpp @@ -1,5 +1,6 @@ #include "rewrite_io_utils.h" +#include #include #include #include @@ -24,7 +25,7 @@ NSQLTranslation::TTranslationSettings CreateViewTranslationSettings(const TStrin return settings; } -TExprNode::TPtr CompileQuery( +TExprNode::TPtr CompileViewQuery( const TString& query, TExprContext& ctx, const TString& cluster @@ -45,12 +46,6 @@ TExprNode::TPtr CompileQuery( return queryGraph; } -bool ContainsNullNode(const TExprNode::TPtr& root) { - return static_cast(FindNode(root, [](const TExprNode::TPtr& node) { - return !node; - })); -} - void AddChild(const TExprNode::TPtr& parent, const TExprNode::TPtr& newChild) { auto childrenToChange = parent->ChildrenList(); childrenToChange.emplace_back(newChild); @@ -137,22 +132,22 @@ TExprNode::TPtr RewriteReadFromView( const TString& query, const TString& cluster ) { - const auto readNode = node->ChildPtr(0); - const auto worldBeforeThisRead = readNode->ChildPtr(0); + const TCoRead readNode(node->ChildPtr(0)); + const auto worldBeforeThisRead = readNode.World().Ptr(); - TExprNode::TPtr queryGraph = FindSavedQueryGraph(readNode); + TExprNode::TPtr queryGraph = FindSavedQueryGraph(readNode.Ptr()); if (!queryGraph) { - queryGraph = CompileQuery(query, ctx, cluster); - if (!queryGraph || ContainsNullNode(queryGraph)) { - ctx.AddError(TIssue(readNode->Pos(ctx), - "The query stored in the view contains errors and cannot be compiled.")); + queryGraph = CompileViewQuery(query, ctx, cluster); + if (!queryGraph) { + ctx.AddError(TIssue(ctx.GetPosition(readNode.Pos()), + "The query stored in the view cannot be compiled.")); return nullptr; } YQL_CLOG(TRACE, ProviderKqp) << "Expression graph of the query stored in the view:\n" << NCommon::ExprToPrettyString(ctx, *queryGraph); InsertExecutionOrderDependencies(queryGraph, worldBeforeThisRead); - SaveQueryGraph(readNode, ctx, queryGraph); + SaveQueryGraph(readNode.Ptr(), ctx, queryGraph); } if (node->IsCallable(RightName)) { diff --git a/ydb/core/kqp/ut/view/view_ut.cpp b/ydb/core/kqp/ut/view/view_ut.cpp index 1988ce52ae02..b6d606b90665 100644 --- a/ydb/core/kqp/ut/view/view_ut.cpp +++ b/ydb/core/kqp/ut/view/view_ut.cpp @@ -12,10 +12,14 @@ using namespace NYdb::NTable; namespace { -void SetEnableViewsFeatureFlag(TKikimrRunner& kikimr) { +void EnableViewsFeatureFlag(TKikimrRunner& kikimr) { kikimr.GetTestServer().GetRuntime()->GetAppData(0).FeatureFlags.SetEnableViews(true); } +void DisableViewsFeatureFlag(TKikimrRunner& kikimr) { + kikimr.GetTestServer().GetRuntime()->GetAppData(0).FeatureFlags.SetEnableViews(false); +} + NKikimrSchemeOp::TViewDescription GetViewDescription(TTestActorRuntime& runtime, const TString& path) { const auto pathQueryResult = Navigate(runtime, runtime.AllocateEdgeActor(), @@ -53,7 +57,7 @@ TString ReadWholeFile(const TString& path) { void ExecuteDataDefinitionQuery(TSession& session, const TString& script) { Cerr << "Executing the following DDL script:\n" << script; - const auto result = session.ExecuteSchemeQuery(script, {}).ExtractValueSync(); + const auto result = session.ExecuteSchemeQuery(script).ExtractValueSync(); UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); } @@ -62,8 +66,7 @@ TDataQueryResult ExecuteDataModificationQuery(TSession& session, const TString& const auto result = session.ExecuteDataQuery( script, - TTxControl::BeginTx(TTxSettings::SerializableRW()).CommitTx(), - {} + TTxControl::BeginTx(TTxSettings::SerializableRW()).CommitTx() ).ExtractValueSync(); UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); @@ -92,7 +95,7 @@ Y_UNIT_TEST_SUITE(TKQPViewTest) { Y_UNIT_TEST(CheckCreatedView) { TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false)); - SetEnableViewsFeatureFlag(kikimr); + EnableViewsFeatureFlag(kikimr); auto& runtime = *kikimr.GetTestServer().GetRuntime(); auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); @@ -111,9 +114,27 @@ Y_UNIT_TEST_SUITE(TKQPViewTest) { UNIT_ASSERT_EQUAL(viewDescription.GetQueryText(), queryInView); } + Y_UNIT_TEST(CreateViewDisabledFeatureFlag) { + TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false)); + auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); + + constexpr const char* path = "/Root/TheView"; + + const TString creationQuery = std::format(R"( + CREATE VIEW `{}` WITH (security_invoker = true) AS SELECT 1; + )", + path + ); + + DisableViewsFeatureFlag(kikimr); + const auto creationResult = session.ExecuteSchemeQuery(creationQuery).ExtractValueSync(); + UNIT_ASSERT(!creationResult.IsSuccess()); + UNIT_ASSERT_STRING_CONTAINS(creationResult.GetIssues().ToString(), "Error: Views are disabled"); + } + Y_UNIT_TEST(InvalidQuery) { TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false)); - SetEnableViewsFeatureFlag(kikimr); + EnableViewsFeatureFlag(kikimr); auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); constexpr const char* path = "/Root/TheView"; @@ -131,14 +152,14 @@ Y_UNIT_TEST_SUITE(TKQPViewTest) { queryInView ); - const auto creationResult = session.ExecuteSchemeQuery(creationQuery, {}).ExtractValueSync(); + const auto creationResult = session.ExecuteSchemeQuery(creationQuery).ExtractValueSync(); UNIT_ASSERT(!creationResult.IsSuccess()); UNIT_ASSERT_STRING_CONTAINS(creationResult.GetIssues().ToString(), "Error: Cannot divide type String and String"); } Y_UNIT_TEST(ListCreatedView) { TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false)); - SetEnableViewsFeatureFlag(kikimr); + EnableViewsFeatureFlag(kikimr); auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); // .sys directory is always present in the `/Root`, that's why we need a subfolder @@ -167,7 +188,7 @@ Y_UNIT_TEST_SUITE(TKQPViewTest) { Y_UNIT_TEST(CreateSameViewTwice) { TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false)); - SetEnableViewsFeatureFlag(kikimr); + EnableViewsFeatureFlag(kikimr); auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); constexpr const char* path = "/Root/TheView"; @@ -189,7 +210,7 @@ Y_UNIT_TEST_SUITE(TKQPViewTest) { Y_UNIT_TEST(DropView) { TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false)); - SetEnableViewsFeatureFlag(kikimr); + EnableViewsFeatureFlag(kikimr); auto& runtime = *kikimr.GetTestServer().GetRuntime(); auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); @@ -213,9 +234,34 @@ Y_UNIT_TEST_SUITE(TKQPViewTest) { ExpectUnknownEntry(runtime, path); } + Y_UNIT_TEST(DropViewDisabledFeatureFlag) { + TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false)); + auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); + + constexpr const char* path = "/Root/TheView"; + + const TString creationQuery = std::format(R"( + CREATE VIEW `{}` WITH (security_invoker = true) AS SELECT 1; + )", + path + ); + EnableViewsFeatureFlag(kikimr); + ExecuteDataDefinitionQuery(session, creationQuery); + + const TString dropQuery = std::format(R"( + DROP VIEW `{}`; + )", + path + ); + DisableViewsFeatureFlag(kikimr); + const auto dropResult = session.ExecuteSchemeQuery(dropQuery).ExtractValueSync(); + UNIT_ASSERT(!dropResult.IsSuccess()); + UNIT_ASSERT_STRING_CONTAINS(dropResult.GetIssues().ToString(), "Error: Views are disabled"); + } + Y_UNIT_TEST(DropSameViewTwice) { TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false)); - SetEnableViewsFeatureFlag(kikimr); + EnableViewsFeatureFlag(kikimr); auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); constexpr const char* path = "/Root/TheView"; @@ -247,7 +293,7 @@ Y_UNIT_TEST_SUITE(TSelectFromViewTest) { Y_UNIT_TEST(OneTable) { TKikimrRunner kikimr; - SetEnableViewsFeatureFlag(kikimr); + EnableViewsFeatureFlag(kikimr); auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); constexpr const char* viewName = "/Root/TheView"; @@ -281,9 +327,37 @@ Y_UNIT_TEST_SUITE(TSelectFromViewTest) { CompareResults(etalonResults, selectFromViewResults); } + Y_UNIT_TEST(DisabledFeatureFlag) { + TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false)); + auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); + + constexpr const char* path = "/Root/TheView"; + + const TString creationQuery = std::format(R"( + CREATE VIEW `{}` WITH (security_invoker = true) AS SELECT 1; + )", + path + ); + EnableViewsFeatureFlag(kikimr); + ExecuteDataDefinitionQuery(session, creationQuery); + + const TString selectQuery = std::format(R"( + SELECT * FROM `{}`; + )", + path + ); + DisableViewsFeatureFlag(kikimr); + const auto selectResult = session.ExecuteDataQuery( + selectQuery, + TTxControl::BeginTx(TTxSettings::SerializableRW()).CommitTx() + ).ExtractValueSync(); + UNIT_ASSERT(!selectResult.IsSuccess()); + UNIT_ASSERT_STRING_CONTAINS(selectResult.GetIssues().ToString(), "Error: Views are disabled"); + } + Y_UNIT_TEST(ReadTestCasesFromFiles) { TKikimrRunner kikimr; - SetEnableViewsFeatureFlag(kikimr); + EnableViewsFeatureFlag(kikimr); auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); InitializeTablesAndSecondaryViews(session); From 504dad86b6f2606644cac43538b293c19ad15902 Mon Sep 17 00:00:00 2001 From: Daniil Demin Date: Thu, 18 Jan 2024 03:50:55 +0000 Subject: [PATCH 5/5] Use a better way to replace nodes in RewriteReadFromView function VisitExpr is unconventional. + Better error printout in unit tests of views. --- ydb/core/kqp/provider/rewrite_io_utils.cpp | 33 +++++++++------------- ydb/core/kqp/ut/view/view_ut.cpp | 10 +++---- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/ydb/core/kqp/provider/rewrite_io_utils.cpp b/ydb/core/kqp/provider/rewrite_io_utils.cpp index 49d0c4a0d6f5..e9262bd91aaf 100644 --- a/ydb/core/kqp/provider/rewrite_io_utils.cpp +++ b/ydb/core/kqp/provider/rewrite_io_utils.cpp @@ -52,14 +52,6 @@ void AddChild(const TExprNode::TPtr& parent, const TExprNode::TPtr& newChild) { parent->ChangeChildrenInplace(std::move(childrenToChange)); } -void ChangeChild(const TExprNode::TPtr& parent, ui32 index, const TExprNode::TPtr& newChild) { - YQL_ENSURE(parent->ChildrenSize() > index); - - auto childrenToChange = parent->ChildrenList(); - childrenToChange[index] = newChild; - parent->ChangeChildrenInplace(std::move(childrenToChange)); -} - TExprNode::TPtr FindSavedQueryGraph(const TExprNode::TPtr& carrier) { if (carrier->ChildrenSize() == 0) { return nullptr; @@ -72,17 +64,18 @@ void SaveQueryGraph(const TExprNode::TPtr& carrier, TExprContext& ctx, const TEx AddChild(carrier, ctx.NewCallable(payload->Pos(), QueryGraphNodeSignature, {payload})); } -void InsertExecutionOrderDependencies(const TExprNode::TPtr& queryGraph, const TExprNode::TPtr& worldBefore) { - VisitExpr( - queryGraph, - nullptr, - [&worldBefore](const TExprNode::TPtr& node) { - if (node->ChildrenSize() > 0 && node->Child(0)->IsWorld()) { - ChangeChild(node, 0, worldBefore); - } - return true; - } - ); +void InsertExecutionOrderDependencies( + TExprNode::TPtr& queryGraph, + const TExprNode::TPtr& worldBefore, + TExprContext& ctx +) { + const auto initialWorldOfTheQuery = FindNode(queryGraph, [](const TExprNode::TPtr& node) { + return node->IsWorld(); + }); + if (!initialWorldOfTheQuery) { + return; + } + queryGraph = ctx.ReplaceNode(std::move(queryGraph), *initialWorldOfTheQuery, worldBefore); } bool CheckTopLevelness(const TExprNode::TPtr& candidateRead, const TExprNode::TPtr& queryGraph) { @@ -146,7 +139,7 @@ TExprNode::TPtr RewriteReadFromView( YQL_CLOG(TRACE, ProviderKqp) << "Expression graph of the query stored in the view:\n" << NCommon::ExprToPrettyString(ctx, *queryGraph); - InsertExecutionOrderDependencies(queryGraph, worldBeforeThisRead); + InsertExecutionOrderDependencies(queryGraph, worldBeforeThisRead, ctx); SaveQueryGraph(readNode.Ptr(), ctx, queryGraph); } diff --git a/ydb/core/kqp/ut/view/view_ut.cpp b/ydb/core/kqp/ut/view/view_ut.cpp index b6d606b90665..fed6b336e75b 100644 --- a/ydb/core/kqp/ut/view/view_ut.cpp +++ b/ydb/core/kqp/ut/view/view_ut.cpp @@ -55,20 +55,18 @@ TString ReadWholeFile(const TString& path) { } void ExecuteDataDefinitionQuery(TSession& session, const TString& script) { - Cerr << "Executing the following DDL script:\n" << script; - const auto result = session.ExecuteSchemeQuery(script).ExtractValueSync(); - UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); + UNIT_ASSERT_C(result.IsSuccess(), "Failed to execute the following DDL script:\n" + << script << "\nThe issues:\n" << result.GetIssues().ToString()); } TDataQueryResult ExecuteDataModificationQuery(TSession& session, const TString& script) { - Cerr << "Executing the following DML script:\n" << script; - const auto result = session.ExecuteDataQuery( script, TTxControl::BeginTx(TTxSettings::SerializableRW()).CommitTx() ).ExtractValueSync(); - UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); + UNIT_ASSERT_C(result.IsSuccess(), "Failed to execute the following DML script:\n" + << script << "\nThe issues:\n" << result.GetIssues().ToString()); return result; }