From 02df54fe6284c3897d4573863ab247c6492e1911 Mon Sep 17 00:00:00 2001 From: Daniil Demin Date: Tue, 13 Feb 2024 06:39:24 +0000 Subject: [PATCH 1/3] Update query cache if the schema version of the views used in this query was updated Add views, whose metadata was loaded during the compilation of the query, info to the prepared query proto and use the list to check (with SchemeCache) if the schema version of the view was updated and a recompilation is needed. --- .../kqp/provider/yql_kikimr_datasource.cpp | 11 +++ .../kqp/session_actor/kqp_query_state.cpp | 11 +++ ydb/core/kqp/session_actor/kqp_query_state.h | 2 + ydb/core/kqp/ut/view/view_ut.cpp | 94 +++++++++++++++++-- ydb/core/protos/kqp_physical.proto | 2 + 5 files changed, 112 insertions(+), 8 deletions(-) diff --git a/ydb/core/kqp/provider/yql_kikimr_datasource.cpp b/ydb/core/kqp/provider/yql_kikimr_datasource.cpp index 4e45f0a21853..76ff5e43a0a2 100644 --- a/ydb/core/kqp/provider/yql_kikimr_datasource.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_datasource.cpp @@ -351,6 +351,17 @@ class TKiSourceLoadTableMetadataTransformer : public TGraphTransformerBase { if (!AddCluster(table, res, input, ctx)) { return TStatus::Error; } + + if (res.Metadata->Kind == EKikimrTableKind::View) { + const auto& viewMetadata = *res.Metadata; + const auto& preparingQuery = SessionCtx->Query().PreparingQuery; + YQL_ENSURE(preparingQuery); + auto* viewInfo = preparingQuery->MutablePhysicalQuery()->MutableViewsInfo()->Add(); + auto* pathId = viewInfo->MutableTableId(); + pathId->SetOwnerId(viewMetadata.PathId.OwnerId()); + pathId->SetTableId(viewMetadata.PathId.TableId()); + viewInfo->SetSchemaVersion(viewMetadata.SchemaVersion); + } } else { TIssueScopeGuard issueScope(ctx.IssueManager, [input, &table, &ctx]() { return MakeIntrusive(TIssue(ctx.GetPosition(input->Pos()), TStringBuilder() diff --git a/ydb/core/kqp/session_actor/kqp_query_state.cpp b/ydb/core/kqp/session_actor/kqp_query_state.cpp index 9889ac5dee3f..cdb748ab5a89 100644 --- a/ydb/core/kqp/session_actor/kqp_query_state.cpp +++ b/ydb/core/kqp/session_actor/kqp_query_state.cpp @@ -71,6 +71,16 @@ bool TKqpQueryState::EnsureTableVersions(const TEvTxProxySchemeCache::TEvNavigat return true; } +void TKqpQueryState::FillViews(const google::protobuf::RepeatedPtrField< ::NKqpProto::TKqpTableInfo>& views) { + for (const auto& view : views) { + const auto& pathId = view.GetTableId(); + const auto schemaVersion = view.GetSchemaVersion(); + auto [it, isInserted] = TableVersions.emplace(TTableId(pathId.GetOwnerId(), pathId.GetTableId()), schemaVersion); + if (!isInserted) { + Y_ENSURE(it->second == schemaVersion); + } + } +} std::unique_ptr TKqpQueryState::BuildNavigateKeySet() { TableVersions.clear(); @@ -78,6 +88,7 @@ std::unique_ptr TKqpQueryState::BuildN for (const auto& tx : PreparedQuery->GetPhysicalQuery().GetTransactions()) { FillTables(tx); } + FillViews(PreparedQuery->GetPhysicalQuery().GetViewsInfo()); auto navigate = MakeHolder(); navigate->DatabaseName = Database; diff --git a/ydb/core/kqp/session_actor/kqp_query_state.h b/ydb/core/kqp/session_actor/kqp_query_state.h index 7766ddd70116..48f32a7342a2 100644 --- a/ydb/core/kqp/session_actor/kqp_query_state.h +++ b/ydb/core/kqp/session_actor/kqp_query_state.h @@ -234,6 +234,8 @@ class TKqpQueryState : public TNonCopyable { } } + void FillViews(const google::protobuf::RepeatedPtrField< ::NKqpProto::TKqpTableInfo>& views); + bool NeedCheckTableVersions() const { return CompileStats.FromCache; } diff --git a/ydb/core/kqp/ut/view/view_ut.cpp b/ydb/core/kqp/ut/view/view_ut.cpp index e6acc8d7a21c..c3eb3bd492dc 100644 --- a/ydb/core/kqp/ut/view/view_ut.cpp +++ b/ydb/core/kqp/ut/view/view_ut.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include @@ -8,6 +9,7 @@ using namespace NKikimr; using namespace NKikimr::NKqp; +using namespace NYdb; using namespace NYdb::NTable; namespace { @@ -75,8 +77,37 @@ TDataQueryResult ExecuteDataModificationQuery(TSession& session, return result; } -TString GetYsonResults(TSession& session, const TString& query, const TExecDataQuerySettings& settings = {}) { - return FormatResultSetYson(ExecuteDataModificationQuery(session, query, settings).GetResultSet(0)); +TValue GetSingleResult(const TDataQueryResult& rawResults) { + auto resultSetParser = rawResults.GetResultSetParser(0); + UNIT_ASSERT(resultSetParser.TryNextRow()); + return resultSetParser.GetValue(0); +} + +TValue GetSingleResult(TSession& session, const TString& query, const TExecDataQuerySettings& settings = {}) { + return GetSingleResult(ExecuteDataModificationQuery(session, query, settings)); +} + +TInstant GetTimestamp(const TValue& value) { + return TValueParser(value).GetTimestamp(); +} + +int GetInteger(const TValue& value) { + return TValueParser(value).GetInt32(); +} + +TMaybe GetFromCacheStat(const TQueryStats& stats) { + const auto& proto = TProtoAccessor::GetProto(stats); + if (!proto.Hascompilation()) { + return Nothing(); + } + return proto.Getcompilation().Getfrom_cache(); +} + +void AssertFromCache(const TMaybe& stats, bool expectedValue) { + UNIT_ASSERT(stats.Defined()); + const auto isFromCache = GetFromCacheStat(*stats); + UNIT_ASSERT_C(isFromCache.Defined(), stats->ToString()); + UNIT_ASSERT_VALUES_EQUAL_C(*isFromCache, expectedValue, stats->ToString()); } void CompareResults(const TDataQueryResult& first, const TDataQueryResult& second) { @@ -384,6 +415,53 @@ Y_UNIT_TEST_SUITE(TSelectFromViewTest) { ExecuteDataDefinitionQuery(session, ReadWholeFile(pathPrefix + "drop_view.sql")); } } + + Y_UNIT_TEST(QueryCacheIsUpdated) { + TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false)); + EnableViewsFeatureFlag(kikimr); + auto session = kikimr.GetTableClient().CreateSession().GetValueSync().GetSession(); + + constexpr const char* viewName = "TheView"; + + const auto getCreationQuery = [&viewName](const char* innerQuery) -> TString { + return std::format(R"( + CREATE VIEW {} WITH (security_invoker = TRUE) AS {}; + )", + viewName, + innerQuery + ); + }; + constexpr const char* firstInnerQuery = "SELECT 1"; + ExecuteDataDefinitionQuery(session, getCreationQuery(firstInnerQuery)); + + const TString selectFromViewQuery = std::format(R"( + SELECT * FROM {}; + )", + viewName + ); + TExecDataQuerySettings queryExecutionSettings; + queryExecutionSettings.KeepInQueryCache(true); + queryExecutionSettings.CollectQueryStats(ECollectQueryStatsMode::Basic); + ExecuteDataModificationQuery(session, selectFromViewQuery, queryExecutionSettings); + // make sure the server side cache is working by calling the same query twice + const auto cachedQueryRawResult = ExecuteDataModificationQuery(session, selectFromViewQuery, queryExecutionSettings); + AssertFromCache(cachedQueryRawResult.GetStats(), true); + UNIT_ASSERT_VALUES_EQUAL(GetInteger(GetSingleResult(cachedQueryRawResult)), 1); + + // recreate the view with a different query inside + ExecuteDataDefinitionQuery(session, std::format(R"( + DROP VIEW {}; + )", + viewName + ) + ); + constexpr const char* secondInnerQuery = "SELECT 2"; + ExecuteDataDefinitionQuery(session, getCreationQuery(secondInnerQuery)); + + const auto secondCallRawResult = ExecuteDataModificationQuery(session, selectFromViewQuery, queryExecutionSettings); + AssertFromCache(secondCallRawResult.GetStats(), false); + UNIT_ASSERT_VALUES_EQUAL(GetInteger(GetSingleResult(secondCallRawResult)), 2); + } } Y_UNIT_TEST_SUITE(TEvaluateExprInViewTest) { @@ -414,9 +492,9 @@ Y_UNIT_TEST_SUITE(TEvaluateExprInViewTest) { TExecDataQuerySettings queryExecutionSettings; queryExecutionSettings.KeepInQueryCache(true); const auto executeTwice = [&](const TString& query) { - return TVector{ - GetYsonResults(session, query, queryExecutionSettings), - GetYsonResults(session, query, queryExecutionSettings) + return TVector{ + GetTimestamp(GetSingleResult(session, query, queryExecutionSettings)), + GetTimestamp(GetSingleResult(session, query, queryExecutionSettings)) }; }; const auto viewResults = executeTwice(selectFromViewQuery); @@ -455,9 +533,9 @@ Y_UNIT_TEST_SUITE(TEvaluateExprInViewTest) { TExecDataQuerySettings queryExecutionSettings; queryExecutionSettings.KeepInQueryCache(true); const auto executeTwice = [&](const TString& query) { - return TVector{ - GetYsonResults(session, query, queryExecutionSettings), - GetYsonResults(session, query, queryExecutionSettings) + return TVector{ + GetTimestamp(GetSingleResult(session, query, queryExecutionSettings)), + GetTimestamp(GetSingleResult(session, query, queryExecutionSettings)) }; }; const auto viewResults = executeTwice(selectFromViewQuery); diff --git a/ydb/core/protos/kqp_physical.proto b/ydb/core/protos/kqp_physical.proto index 9ee7b069ed04..365635f5f808 100644 --- a/ydb/core/protos/kqp_physical.proto +++ b/ydb/core/protos/kqp_physical.proto @@ -507,4 +507,6 @@ message TKqpPhyQuery { bool HasUncommittedChangesRead = 9; string QueryDiagnostics = 10; + + repeated TKqpTableInfo ViewsInfo = 11; } From e81137195593c73193c707f0b3e2697a4ef1de45 Mon Sep 17 00:00:00 2001 From: Daniil Demin Date: Fri, 16 Feb 2024 09:34:14 +0000 Subject: [PATCH 2/3] Bug fix for CREATE VIEW cases (which also load view metadata, but there is no PreparedQuery) --- ydb/core/kqp/provider/yql_kikimr_datasource.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ydb/core/kqp/provider/yql_kikimr_datasource.cpp b/ydb/core/kqp/provider/yql_kikimr_datasource.cpp index 76ff5e43a0a2..8e66a6a210c5 100644 --- a/ydb/core/kqp/provider/yql_kikimr_datasource.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_datasource.cpp @@ -352,10 +352,11 @@ class TKiSourceLoadTableMetadataTransformer : public TGraphTransformerBase { return TStatus::Error; } - if (res.Metadata->Kind == EKikimrTableKind::View) { + if (const auto& preparingQuery = SessionCtx->Query().PreparingQuery; + preparingQuery + && res.Metadata->Kind == EKikimrTableKind::View + ) { const auto& viewMetadata = *res.Metadata; - const auto& preparingQuery = SessionCtx->Query().PreparingQuery; - YQL_ENSURE(preparingQuery); auto* viewInfo = preparingQuery->MutablePhysicalQuery()->MutableViewsInfo()->Add(); auto* pathId = viewInfo->MutableTableId(); pathId->SetOwnerId(viewMetadata.PathId.OwnerId()); From db550498505e61d0eeefd2d6eafc6f16d6514915 Mon Sep 17 00:00:00 2001 From: Daniil Demin Date: Fri, 16 Feb 2024 15:21:45 +0000 Subject: [PATCH 3/3] Rename a variable --- ydb/core/kqp/provider/yql_kikimr_datasource.cpp | 2 +- ydb/core/kqp/session_actor/kqp_query_state.cpp | 2 +- ydb/core/protos/kqp_physical.proto | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ydb/core/kqp/provider/yql_kikimr_datasource.cpp b/ydb/core/kqp/provider/yql_kikimr_datasource.cpp index 8e66a6a210c5..af95adf40d5a 100644 --- a/ydb/core/kqp/provider/yql_kikimr_datasource.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_datasource.cpp @@ -357,7 +357,7 @@ class TKiSourceLoadTableMetadataTransformer : public TGraphTransformerBase { && res.Metadata->Kind == EKikimrTableKind::View ) { const auto& viewMetadata = *res.Metadata; - auto* viewInfo = preparingQuery->MutablePhysicalQuery()->MutableViewsInfo()->Add(); + auto* viewInfo = preparingQuery->MutablePhysicalQuery()->MutableViewInfos()->Add(); auto* pathId = viewInfo->MutableTableId(); pathId->SetOwnerId(viewMetadata.PathId.OwnerId()); pathId->SetTableId(viewMetadata.PathId.TableId()); diff --git a/ydb/core/kqp/session_actor/kqp_query_state.cpp b/ydb/core/kqp/session_actor/kqp_query_state.cpp index cdb748ab5a89..e68dab1cc975 100644 --- a/ydb/core/kqp/session_actor/kqp_query_state.cpp +++ b/ydb/core/kqp/session_actor/kqp_query_state.cpp @@ -88,7 +88,7 @@ std::unique_ptr TKqpQueryState::BuildN for (const auto& tx : PreparedQuery->GetPhysicalQuery().GetTransactions()) { FillTables(tx); } - FillViews(PreparedQuery->GetPhysicalQuery().GetViewsInfo()); + FillViews(PreparedQuery->GetPhysicalQuery().GetViewInfos()); auto navigate = MakeHolder(); navigate->DatabaseName = Database; diff --git a/ydb/core/protos/kqp_physical.proto b/ydb/core/protos/kqp_physical.proto index 365635f5f808..98351bfe1f1e 100644 --- a/ydb/core/protos/kqp_physical.proto +++ b/ydb/core/protos/kqp_physical.proto @@ -508,5 +508,5 @@ message TKqpPhyQuery { string QueryDiagnostics = 10; - repeated TKqpTableInfo ViewsInfo = 11; + repeated TKqpTableInfo ViewInfos = 11; }