From ab2e3bc180a58eb4b62735b33b034d5fa67cf85b Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Thu, 7 Nov 2024 15:07:09 +0300 Subject: [PATCH 01/22] Initial commit --- .../grpc_services/query/rpc_execute_query.cpp | 7 +- .../grpc_services/rpc_execute_data_query.cpp | 6 +- ydb/core/kqp/common/events/query.h | 4 +- ydb/core/kqp/common/kqp_event_impl.cpp | 6 +- ydb/core/kqp/ut/query/kqp_query_ut.cpp | 66 +++++++++++++++++++ ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp | 49 ++++++++++++++ ydb/public/api/protos/ydb_query.proto | 5 ++ ydb/public/api/protos/ydb_table.proto | 3 + .../ydb_cli/commands/ydb_service_table.cpp | 24 +++++++ .../lib/ydb_cli/commands/ydb_service_table.h | 1 + .../sdk/cpp/client/ydb_query/client.cpp | 5 ++ ydb/public/sdk/cpp/client/ydb_query/client.h | 14 +++- .../cpp/client/ydb_query/impl/exec_query.cpp | 20 ++++-- ydb/public/sdk/cpp/client/ydb_query/query.h | 1 + .../cpp/client/ydb_table/impl/table_client.h | 9 ++- ydb/public/sdk/cpp/client/ydb_table/table.cpp | 9 ++- ydb/public/sdk/cpp/client/ydb_table/table.h | 7 +- 17 files changed, 220 insertions(+), 16 deletions(-) diff --git a/ydb/core/grpc_services/query/rpc_execute_query.cpp b/ydb/core/grpc_services/query/rpc_execute_query.cpp index dcf0d45b38b0..00601798ff45 100644 --- a/ydb/core/grpc_services/query/rpc_execute_query.cpp +++ b/ydb/core/grpc_services/query/rpc_execute_query.cpp @@ -267,6 +267,8 @@ class TExecuteQueryRPC : public TActorBootstrapped { .SetSupportStreamTrailingResult(true) .SetOutputChunkMaxSize(req->response_part_limit_bytes()); + assert(req->Getcollect_full_diagnostics()); + auto ev = MakeHolder( QueryAction, queryType, @@ -281,7 +283,8 @@ class TExecuteQueryRPC : public TActorBootstrapped { cachePolicy, nullptr, // operationParams settings, - req->pool_id()); + req->pool_id(), + req->Getcollect_full_diagnostics()); ev->SetProgressStatsPeriod(TDuration::MilliSeconds(req->stats_period_ms())); @@ -422,6 +425,8 @@ class TExecuteQueryRPC : public TActorBootstrapped { hasTrailingMessage = true; response.mutable_tx_meta()->set_id(kqpResponse.GetTxMeta().id()); } + assert(!kqpResponse.GetQueryDiagnostics().empty()); + response.set_query_full_diagnostics(kqpResponse.GetQueryDiagnostics()); } if (hasTrailingMessage) { diff --git a/ydb/core/grpc_services/rpc_execute_data_query.cpp b/ydb/core/grpc_services/rpc_execute_data_query.cpp index f134adf39964..d9c6654a9926 100644 --- a/ydb/core/grpc_services/rpc_execute_data_query.cpp +++ b/ydb/core/grpc_services/rpc_execute_data_query.cpp @@ -145,7 +145,10 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActorparameters(), req->collect_stats(), req->has_query_cache_policy() ? &req->query_cache_policy() : nullptr, - req->has_operation_params() ? &req->operation_params() : nullptr); + req->has_operation_params() ? &req->operation_params() : nullptr, + NKqp::NPrivateEvents::TQueryRequestSettings(), + "", + req->Getcollect_full_diagnostics()); ReportCostInfo_ = req->operation_params().report_cost_info() == Ydb::FeatureFlag::ENABLED; @@ -203,6 +206,7 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActorinsert({queryParameter.GetName(), parameterType}); } } + queryResult->set_query_full_diagnostics(kqpResponse.GetQueryDiagnostics()); } catch (const std::exception& ex) { NYql::TIssues issues; issues.AddIssue(NYql::ExceptionToIssue(ex)); diff --git a/ydb/core/kqp/common/events/query.h b/ydb/core/kqp/common/events/query.h index 19aad90211f2..a246a4a0d939 100644 --- a/ydb/core/kqp/common/events/query.h +++ b/ydb/core/kqp/common/events/query.h @@ -68,7 +68,8 @@ struct TEvQueryRequest: public NActors::TEventLocalSetUsePublicResponseDataFormat(true); @@ -395,6 +396,7 @@ struct TEvQueryRequest: public NActors::TEventLocal UserRequestContext; TDuration ProgressStatsPeriod; std::optional PoolConfig; + bool CollectFullDiagnostics = false; }; struct TEvDataQueryStreamPart: public TEventPBGetDatabaseName().GetOrElse(""))) @@ -35,6 +36,7 @@ TEvKqp::TEvQueryRequest::TEvQueryRequest( , QueryCachePolicy(queryCachePolicy) , HasOperationParams(operationParams) , QuerySettings(querySettings) + , CollectFullDiagnostics(collectFullDiagnostics) { if (HasOperationParams) { OperationTimeout = GetDuration(operationParams->operation_timeout()); @@ -107,6 +109,8 @@ void TEvKqp::TEvQueryRequest::PrepareRemote() const { Record.MutableRequest()->SetIsInternalCall(RequestCtx->IsInternalCall()); Record.MutableRequest()->SetOutputChunkMaxSize(QuerySettings.OutputChunkMaxSize); + Record.MutableRequest()->SetCollectDiagnostics(CollectFullDiagnostics); + RequestCtx.reset(); } } diff --git a/ydb/core/kqp/ut/query/kqp_query_ut.cpp b/ydb/core/kqp/ut/query/kqp_query_ut.cpp index 88ed1d860074..ffecb6ea2ee6 100644 --- a/ydb/core/kqp/ut/query/kqp_query_ut.cpp +++ b/ydb/core/kqp/ut/query/kqp_query_ut.cpp @@ -179,6 +179,72 @@ Y_UNIT_TEST_SUITE(KqpQuery) { UNIT_ASSERT_VALUES_EQUAL(counters.RecompileRequestGet()->Val(), 1); } + Y_UNIT_TEST(ExecuteDataQueryCollectFullDiagnostics) { + auto setting = NKikimrKqp::TKqpSetting(); + auto serverSettings = TKikimrSettings() + .SetKqpSettings({setting}); + + TKikimrRunner kikimr(serverSettings); + auto db = kikimr.GetTableClient(); + auto session = db.CreateSession().GetValueSync().GetSession(); + + { + UNIT_ASSERT(session.ExecuteSchemeQuery(R"( + CREATE TABLE `/Root/TestTable` ( + Key Uint64, + Value String, + PRIMARY KEY (Key) + ); + )").GetValueSync().IsSuccess()); + } + + { + const TString query(Q1_(R"( + SELECT * FROM `/Root/TestTable`; + )")); + + { + auto settings = TExecDataQuerySettings(); + settings.CollectFullDiagnostics(true); + + auto result = session.ExecuteDataQuery(query, TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); + + UNIT_ASSERT_C(!result.GetDiagnostics().empty(), "Query result diagnostics is empty"); + + TStringStream in; + in << result.GetDiagnostics(); + NJson::TJsonValue value; + ReadJsonTree(&in, &value); + + UNIT_ASSERT_C(value.IsMap(), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("version"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_text"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Diagnostics: table_metadata type should be an array"); + UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_plan"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Diagnostics"); + } + + { + auto settings = TExecDataQuerySettings(); + + auto result = session.ExecuteDataQuery(query, TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); + + UNIT_ASSERT_C(result.GetDiagnostics().empty(), "Query result diagnostics should be empty, but it's not"); + } + } + } + Y_UNIT_TEST(QueryCachePermissionsLoss) { TKikimrRunner kikimr; auto db = kikimr.GetTableClient(); diff --git a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp index b27852b6566a..a357617ee120 100644 --- a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp +++ b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp @@ -650,6 +650,55 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { } } + Y_UNIT_TEST(ExecuteCollectFullDiagnostics) { + auto kikimr = DefaultKikimrRunner(); + auto db = kikimr.GetQueryClient(); + + { + TExecuteQuerySettings settings; + settings.CollectFullDiagnostics(true); + + auto result = db.ExecuteQuery(R"( + SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; + )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT_C(!result.GetDiagnostics().empty(), "Query result diagnostics is empty"); + + TStringStream in; + in << result.GetDiagnostics(); + NJson::TJsonValue value; + ReadJsonTree(&in, &value); + + UNIT_ASSERT_C(value.IsMap(), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("version"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_text"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Diagnostics: table_metadata type should be an array"); + UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_plan"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Diagnostics"); + } + + { + TExecuteQuerySettings settings; + settings.CollectFullDiagnostics(true); + + auto result = db.ExecuteQuery(R"( + SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; + )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); + + UNIT_ASSERT_C(result.GetDiagnostics().empty(), "Query result diagnostics should be empty, but it's not"); + } + } + void CheckQueryResult(TExecuteQueryResult result) { UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); UNIT_ASSERT_VALUES_EQUAL(result.GetResultSets().size(), 1); diff --git a/ydb/public/api/protos/ydb_query.proto b/ydb/public/api/protos/ydb_query.proto index b065520458cf..0427f6652e9a 100644 --- a/ydb/public/api/protos/ydb_query.proto +++ b/ydb/public/api/protos/ydb_query.proto @@ -181,6 +181,8 @@ message ExecuteQueryRequest { // When query statistics are enabled (stats_mode != STATS_MODE_NONE), by default statistics will be sent only once after query execution is finished. // In case when stats_period_ms is specified and is non-zero, query statistics will be additionally sent every stats_period_ms milliseconds beginning from the start of query execution. int64 stats_period_ms = 11 [(Ydb.value) = ">= 0"]; + + bool collect_full_diagnostics = 12; } message ResultSetMeta { @@ -200,6 +202,9 @@ message ExecuteQueryResponsePart { Ydb.TableStats.QueryStats exec_stats = 5; TransactionMeta tx_meta = 6; + + // Full query diagnostics + string query_full_diagnostics = 7; } message ExecuteScriptRequest { diff --git a/ydb/public/api/protos/ydb_table.proto b/ydb/public/api/protos/ydb_table.proto index 7a91977050ce..2552f5c6987d 100644 --- a/ydb/public/api/protos/ydb_table.proto +++ b/ydb/public/api/protos/ydb_table.proto @@ -926,6 +926,7 @@ message ExecuteDataQueryRequest { QueryCachePolicy query_cache_policy = 5; Ydb.Operations.OperationParams operation_params = 6; QueryStatsCollection.Mode collect_stats = 7; + bool collect_full_diagnostics = 8; } message ExecuteDataQueryResponse { @@ -968,6 +969,8 @@ message ExecuteQueryResult { QueryMeta query_meta = 3; // Query execution statistics Ydb.TableStats.QueryStats query_stats = 4; + // Full query diagnostics + string query_full_diagnostics = 5; } // Explain data query diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index 08437be17a18..1a92ba0572bb 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -365,6 +365,8 @@ void TCommandExecuteQuery::Config(TConfig& config) { config.Opts->AddLongOption('q', "query", "Text of query to execute").RequiredArgument("[String]").StoreResult(&Query); config.Opts->AddLongOption('f', "file", "Path to file with query text to execute") .RequiredArgument("PATH").StoreResult(&QueryFile); + config.Opts->AddLongOption("collect-diagnostics", "Collects diagnostics and saves it to file") + .StoreTrue(&CollectFullDiagnostics); AddOutputFormats(config, { EDataFormat::Pretty, @@ -432,6 +434,9 @@ int TCommandExecuteQuery::ExecuteDataQuery(TConfig& config) { NTable::TExecDataQuerySettings settings; settings.KeepInQueryCache(true); settings.CollectQueryStats(ParseQueryStatsModeOrThrow(CollectStatsMode, defaultStatsMode)); + if (CollectFullDiagnostics) { + settings.CollectFullDiagnostics(true); + } NTable::TTxSettings txSettings; if (TxMode) { @@ -516,6 +521,11 @@ void TCommandExecuteQuery::PrintDataQueryResponse(NTable::TDataQueryResult& resu { Cout << Endl << "Flame graph is available for full or profile stats only" << Endl; } + if (CollectFullDiagnostics) + { + TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); + file << result.GetDiagnostics(); + } } int TCommandExecuteQuery::ExecuteSchemeQuery(TConfig& config) { @@ -558,6 +568,9 @@ namespace { if (timeout.has_value()) { settings.ClientTimeout(*timeout); } + if (CollectFullDiagnostics) { + settings.CollectFullDiagnostics(true); + } return settings; } else if constexpr (std::is_same_v) { const auto defaultStatsMode = basicStats @@ -568,6 +581,9 @@ namespace { if (timeout.has_value()) { settings.ClientTimeout(*timeout); } + if (CollectFullDiagnostics) { + settings.CollectFullDiagnostics(true); + } return settings; } Y_UNREACHABLE(); @@ -753,6 +769,8 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { fullStats = queryStats.GetPlan(); } } + + if () } } // TResultSetPrinter destructor should be called before printing stats @@ -767,6 +785,12 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { queryPlanPrinter.Print(TString{*fullStats}); } + if (CollectFullDiagnostics) + { + TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); + file << result.GetDiagnostics(); + } + PrintFlameGraph(fullStats); if (IsInterrupted()) { diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.h b/ydb/public/lib/ydb_cli/commands/ydb_service_table.h index 041543abb3ef..7fe5754ebf52 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.h @@ -123,6 +123,7 @@ class TCommandExecuteQuery : public TTableCommand, TCommandQueryBase, TCommandWi TString TxMode; TString QueryType; bool BasicStats = false; + bool CollectFullDiagnostics = false; }; class TCommandExplain : public TTableCommand, public TCommandWithOutput, TCommandQueryBase, TInterruptibleCommand { diff --git a/ydb/public/sdk/cpp/client/ydb_query/client.cpp b/ydb/public/sdk/cpp/client/ydb_query/client.cpp index 68dcb25415d9..8756e5f78f68 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/client.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/client.cpp @@ -734,4 +734,9 @@ TResultSetParser TExecuteQueryResult::GetResultSetParser(size_t resultIndex) con return TResultSetParser(GetResultSet(resultIndex)); } +const TString& TExecuteQueryResult::GetDiagnostics() const { + CheckStatusOk("TExecuteQueryResult::GetDiagnostics"); + return Diagnostics_; +} + } // namespace NYdb::NQuery diff --git a/ydb/public/sdk/cpp/client/ydb_query/client.h b/ydb/public/sdk/cpp/client/ydb_query/client.h index cea936c2028f..8d2de6be05d5 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/client.h +++ b/ydb/public/sdk/cpp/client/ydb_query/client.h @@ -216,23 +216,26 @@ class TExecuteQueryPart : public TStreamPartStatus { ui64 GetResultSetIndex() const { return ResultSetIndex_; } const TResultSet& GetResultSet() const { return *ResultSet_; } TResultSet ExtractResultSet() { return std::move(*ResultSet_); } + const TString& GetDiagnostics() const { return Diagnostics_; } const TMaybe& GetStats() const { return Stats_; } const TMaybe& GetTransaction() const { return Transaction_; } - TExecuteQueryPart(TStatus&& status, TMaybe&& queryStats, TMaybe&& tx) + TExecuteQueryPart(TStatus&& status, TMaybe&& queryStats, TMaybe&& tx, TString&& diagnostics) : TStreamPartStatus(std::move(status)) , Stats_(std::move(queryStats)) , Transaction_(std::move(tx)) + , Diagnostics_(std::move(diagnostics)) {} TExecuteQueryPart(TStatus&& status, TResultSet&& resultSet, i64 resultSetIndex, - TMaybe&& queryStats, TMaybe&& tx) + TMaybe&& queryStats, TMaybe&& tx, TString&& diagnostics) : TStreamPartStatus(std::move(status)) , ResultSet_(std::move(resultSet)) , ResultSetIndex_(resultSetIndex) , Stats_(std::move(queryStats)) , Transaction_(std::move(tx)) + , Diagnostics_(std::move(diagnostics)) {} private: @@ -240,6 +243,7 @@ class TExecuteQueryPart : public TStreamPartStatus { i64 ResultSetIndex_ = 0; TMaybe Stats_; TMaybe Transaction_; + TString Diagnostics_; }; class TExecuteQueryResult : public TStatus { @@ -252,22 +256,26 @@ class TExecuteQueryResult : public TStatus { TMaybe GetTransaction() const {return Transaction_; } + const TString& GetDiagnostics() const; + TExecuteQueryResult(TStatus&& status) : TStatus(std::move(status)) {} TExecuteQueryResult(TStatus&& status, TVector&& resultSets, - TMaybe&& stats, TMaybe&& tx) + TMaybe&& stats, TMaybe&& tx, TString&& diagnostics) : TStatus(std::move(status)) , ResultSets_(std::move(resultSets)) , Stats_(std::move(stats)) , Transaction_(std::move(tx)) + , Diagnostics_(std::move(diagnostics)) {} private: TVector ResultSets_; TMaybe Stats_; TMaybe Transaction_; + TString Diagnostics_; }; } // namespace NYdb::NQuery diff --git a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp index 971712a474d5..37193323bf1f 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp @@ -70,13 +70,14 @@ class TExecuteQueryIterator::TReaderImpl { auto readCb = [self, promise](TGRpcStatus&& grpcStatus) mutable { if (!grpcStatus.Ok()) { self->Finished_ = true; - promise.SetValue({TStatus(TPlainStatus(grpcStatus, self->Endpoint_)), {}, {}}); + promise.SetValue({TStatus(TPlainStatus(grpcStatus, self->Endpoint_)), {}, {}, ""}); } else { NYql::TIssues issues; NYql::IssuesFromMessage(self->Response_.issues(), issues); EStatus clientStatus = static_cast(self->Response_.status()); TPlainStatus plainStatus{clientStatus, std::move(issues), self->Endpoint_, {}}; TStatus status{std::move(plainStatus)}; + TString diagnostics; TMaybe stats; TMaybe tx; @@ -88,16 +89,19 @@ class TExecuteQueryIterator::TReaderImpl { tx = TTransaction(self->Session_.GetRef(), self->Response_.tx_meta().id()); } + diagnostics = self->Response_.query_full_diagnostics(); + if (self->Response_.has_result_set()) { promise.SetValue({ std::move(status), TResultSet(std::move(*self->Response_.mutable_result_set())), self->Response_.result_set_index(), std::move(stats), - std::move(tx) + std::move(tx), + std::move(diagnostics) }); } else { - promise.SetValue({std::move(status), std::move(stats), std::move(tx)}); + promise.SetValue({std::move(status), std::move(stats), std::move(tx), std::move(diagnostics)}); } } }; @@ -148,6 +152,7 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { TVector ResultSets_; TMaybe Stats_; TMaybe Tx_; + TString Diagnostics_; void Next() { TPtr self(this); @@ -181,10 +186,11 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { TStatus(EStatus::SUCCESS, NYql::TIssues(std::move(issues))), std::move(resultSets), std::move(stats), - std::move(tx) + std::move(tx), + {} )); } else { - self->Promise_.SetValue(TExecuteQueryResult(std::move(part), {}, std::move(stats), {})); + self->Promise_.SetValue(TExecuteQueryResult(std::move(part), {}, std::move(stats), {}, {})); } return; @@ -213,6 +219,8 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { self->Tx_ = tx; } + self->Diagnostics_ = part.GetDiagnostics(); + self->Next(); }); } @@ -243,6 +251,8 @@ TFuture> StreamExecuteQueryIm request.set_response_part_limit_bytes(*settings.OutputChunkMaxSize_); } + request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); + if (txControl.HasTx()) { auto requestTxControl = request.mutable_tx_control(); requestTxControl->set_commit_tx(txControl.CommitTx_); diff --git a/ydb/public/sdk/cpp/client/ydb_query/query.h b/ydb/public/sdk/cpp/client/ydb_query/query.h index 3736def97dce..a2141a7ad571 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/query.h +++ b/ydb/public/sdk/cpp/client/ydb_query/query.h @@ -76,6 +76,7 @@ struct TExecuteQuerySettings : public TRequestSettings { FLUENT_SETTING_DEFAULT_DEPRECATED(EStatsMode, StatsMode, EStatsMode::None); FLUENT_SETTING_OPTIONAL_DEPRECATED(bool, ConcurrentResultSets); FLUENT_SETTING_DEPRECATED(TString, ResourcePool); + FLUENT_SETTING_DEFAULT_DEPRECATED(bool, CollectFullDiagnostics, false); }; struct TBeginTxSettings : public TRequestSettings {}; diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h index 04e168d20f31..7cfa440067c2 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h +++ b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h @@ -188,7 +188,8 @@ class TTableClient::TImpl: public TClientImplCommon, public txControl.Tx_, Nothing(), false, - Nothing())); + Nothing(), + {})); } return ExecuteDataQueryInternal(session, query, txControl, params, settings, fromCache); @@ -213,6 +214,7 @@ class TTableClient::TImpl: public TClientImplCommon, public } request.set_collect_stats(GetStatsCollectionMode(settings.CollectQueryStats_)); + request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); SetQuery(query, request.mutable_query()); CollectQuerySize(query, QuerySizeHistogram); @@ -240,6 +242,7 @@ class TTableClient::TImpl: public TClientImplCommon, public TMaybe tx; TMaybe dataQuery; TMaybe queryStats; + TString diagnostics; auto queryText = GetQueryText(query); if (any) { @@ -264,6 +267,8 @@ class TTableClient::TImpl: public TClientImplCommon, public if (result.has_query_stats()) { queryStats = TQueryStats(result.query_stats()); } + + diagnostics = result.query_full_diagnostics(); } if (keepInCache && dataQuery && queryText) { @@ -271,7 +276,7 @@ class TTableClient::TImpl: public TClientImplCommon, public } TDataQueryResult dataQueryResult(TStatus(std::move(status)), - std::move(res), tx, dataQuery, fromCache, queryStats); + std::move(res), tx, dataQuery, fromCache, queryStats, std::move(diagnostics)); delete sessionPtr; tx.Clear(); diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.cpp b/ydb/public/sdk/cpp/client/ydb_table/table.cpp index 39708e380077..2d47c582cd44 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/table.cpp @@ -2158,13 +2158,15 @@ TTableDescription TDescribeTableResult::GetTableDescription() const { //////////////////////////////////////////////////////////////////////////////// TDataQueryResult::TDataQueryResult(TStatus&& status, TVector&& resultSets, - const TMaybe& transaction, const TMaybe& dataQuery, bool fromCache, const TMaybe &queryStats) + const TMaybe& transaction, const TMaybe& dataQuery, bool fromCache, const TMaybe &queryStats, + TString&& diagnostics) : TStatus(std::move(status)) , Transaction_(transaction) , ResultSets_(std::move(resultSets)) , DataQuery_(dataQuery) , FromCache_(fromCache) , QueryStats_(queryStats) + , Diagnostics_(std::move(diagnostics)) {} const TVector& TDataQueryResult::GetResultSets() const { @@ -2211,6 +2213,11 @@ const TString TDataQueryResult::GetQueryPlan() const { } } +const TString& TDataQueryResult::GetDiagnostics() const { + CheckStatusOk("TDataQueryResult::GetDiagnostics"); + return Diagnostics_; +} + //////////////////////////////////////////////////////////////////////////////// TBeginTransactionResult::TBeginTransactionResult(TStatus&& status, TTransaction transaction) diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.h b/ydb/public/sdk/cpp/client/ydb_table/table.h index 155e4a949565..1be9803bbf1f 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.h +++ b/ydb/public/sdk/cpp/client/ydb_table/table.h @@ -1726,6 +1726,8 @@ struct TExecDataQuerySettings : public TOperationRequestSettings {}; @@ -2028,7 +2030,7 @@ class TDescribeTableResult : public NScheme::TDescribePathResult { class TDataQueryResult : public TStatus { public: TDataQueryResult(TStatus&& status, TVector&& resultSets, const TMaybe& transaction, - const TMaybe& dataQuery, bool fromCache, const TMaybe& queryStats); + const TMaybe& dataQuery, bool fromCache, const TMaybe& queryStats, TString&& diagnostics); const TVector& GetResultSets() const; TVector ExtractResultSets() &&; @@ -2045,12 +2047,15 @@ class TDataQueryResult : public TStatus { const TString GetQueryPlan() const; + const TString& GetDiagnostics() const; + private: TMaybe Transaction_; TVector ResultSets_; TMaybe DataQuery_; bool FromCache_; TMaybe QueryStats_; + TString Diagnostics_; }; class TReadTableSnapshot { From 298fce3b1920621458ba40f038d0dbc19fc50b25 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Thu, 7 Nov 2024 20:42:24 +0300 Subject: [PATCH 02/22] Fixes --- ydb/core/grpc_services/query/rpc_execute_query.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/ydb/core/grpc_services/query/rpc_execute_query.cpp b/ydb/core/grpc_services/query/rpc_execute_query.cpp index 00601798ff45..95d06d4984a8 100644 --- a/ydb/core/grpc_services/query/rpc_execute_query.cpp +++ b/ydb/core/grpc_services/query/rpc_execute_query.cpp @@ -267,8 +267,6 @@ class TExecuteQueryRPC : public TActorBootstrapped { .SetSupportStreamTrailingResult(true) .SetOutputChunkMaxSize(req->response_part_limit_bytes()); - assert(req->Getcollect_full_diagnostics()); - auto ev = MakeHolder( QueryAction, queryType, @@ -425,7 +423,6 @@ class TExecuteQueryRPC : public TActorBootstrapped { hasTrailingMessage = true; response.mutable_tx_meta()->set_id(kqpResponse.GetTxMeta().id()); } - assert(!kqpResponse.GetQueryDiagnostics().empty()); response.set_query_full_diagnostics(kqpResponse.GetQueryDiagnostics()); } From 9acd1eac231594bdf9bb64dbba0159890bea27b7 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Thu, 14 Nov 2024 07:53:25 +0300 Subject: [PATCH 03/22] Fixes --- ydb/core/kqp/common/events/query.h | 6 +++--- ydb/core/kqp/common/kqp_event_impl.cpp | 4 +++- ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp | 4 +++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/ydb/core/kqp/common/events/query.h b/ydb/core/kqp/common/events/query.h index a246a4a0d939..a50cda93f0d1 100644 --- a/ydb/core/kqp/common/events/query.h +++ b/ydb/core/kqp/common/events/query.h @@ -69,7 +69,7 @@ struct TEvQueryRequest: public NActors::TEventLocal collectFullDiagnostics = std::nullopt); TEvQueryRequest() { Record.MutableRequest()->SetUsePublicResponseDataFormat(true); @@ -283,7 +283,7 @@ struct TEvQueryRequest: public NActors::TEventLocal UserRequestContext; TDuration ProgressStatsPeriod; std::optional PoolConfig; - bool CollectFullDiagnostics = false; + std::optional CollectFullDiagnostics = std::nullopt; }; struct TEvDataQueryStreamPart: public TEventPBSetIsInternalCall(RequestCtx->IsInternalCall()); Record.MutableRequest()->SetOutputChunkMaxSize(QuerySettings.OutputChunkMaxSize); - Record.MutableRequest()->SetCollectDiagnostics(CollectFullDiagnostics); + if (CollectFullDiagnostics.has_value()) { + Record.MutableRequest()->SetCollectDiagnostics(CollectFullDiagnostics.value()); + } RequestCtx.reset(); } diff --git a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp index 37193323bf1f..37151814331d 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp @@ -172,10 +172,12 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { TVector issues; TVector resultProtos; TMaybe tx; + TString diagnostics; std::swap(self->Issues_, issues); std::swap(self->ResultSets_, resultProtos); std::swap(self->Tx_, tx); + std::swap(self->Diagnostics_, diagnostics); TVector resultSets; for (auto& proto : resultProtos) { @@ -187,7 +189,7 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { std::move(resultSets), std::move(stats), std::move(tx), - {} + std::move(diagnostics) )); } else { self->Promise_.SetValue(TExecuteQueryResult(std::move(part), {}, std::move(stats), {}, {})); From 22b62b8c8e7925bd93991b328b8bd8e3707462e0 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Thu, 14 Nov 2024 08:04:36 +0300 Subject: [PATCH 04/22] Fixes --- ydb/core/kqp/common/kqp_event_impl.cpp | 2 +- .../lib/ydb_cli/commands/ydb_service_table.cpp | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/ydb/core/kqp/common/kqp_event_impl.cpp b/ydb/core/kqp/common/kqp_event_impl.cpp index e564cadd1ace..4d23e043a45d 100644 --- a/ydb/core/kqp/common/kqp_event_impl.cpp +++ b/ydb/core/kqp/common/kqp_event_impl.cpp @@ -20,7 +20,7 @@ TEvKqp::TEvQueryRequest::TEvQueryRequest( const ::Ydb::Operations::OperationParams* operationParams, const TQueryRequestSettings& querySettings, const TString& poolId, - bool collectFullDiagnostics) + std::optional collectFullDiagnostics) : RequestCtx(ctx) , RequestActorId(requestActorId) , Database(CanonizePath(ctx->GetDatabaseName().GetOrElse(""))) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index 1a92ba0572bb..7eca82e02576 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -558,7 +558,7 @@ namespace { NQuery::TExecuteQuerySettings>; template - auto GetSettings(const TString& collectStatsMode, const bool basicStats, std::optional timeout) { + auto GetSettings(const TString& collectStatsMode, const bool basicStats, std::optional timeout, bool collectFullDiagnostics) { if constexpr (std::is_same_v) { const auto defaultStatsMode = basicStats ? NTable::ECollectQueryStatsMode::Basic @@ -568,7 +568,7 @@ namespace { if (timeout.has_value()) { settings.ClientTimeout(*timeout); } - if (CollectFullDiagnostics) { + if (collectFullDiagnostics) { settings.CollectFullDiagnostics(true); } return settings; @@ -581,7 +581,7 @@ namespace { if (timeout.has_value()) { settings.ClientTimeout(*timeout); } - if (CollectFullDiagnostics) { + if (collectFullDiagnostics) { settings.CollectFullDiagnostics(true); } return settings; @@ -651,7 +651,7 @@ namespace { } Y_UNREACHABLE(); } - + template const NQuery::TExecStats& GetStats(const TQueryPart& part) { if constexpr (std::is_same_v) { @@ -690,7 +690,7 @@ int TCommandExecuteQuery::ExecuteQueryImpl(TConfig& config) { if (OperationTimeout) { optTimeout = TDuration::MilliSeconds(FromString(OperationTimeout)); } - const auto settings = GetSettings(CollectStatsMode, BasicStats, optTimeout); + const auto settings = GetSettings(CollectStatsMode, BasicStats, optTimeout, CollectFullDiagnostics); TAsyncPartIterator asyncResult; SetInterruptHandlers(); @@ -748,6 +748,7 @@ template bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { TMaybe stats; std::optional fullStats; + TString diagnostics; { TResultSetPrinter printer(OutputFormat, &IsInterrupted); @@ -770,7 +771,7 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { } } - if () + diagnostics = streamPart.GetDiagnostics(); } } // TResultSetPrinter destructor should be called before printing stats @@ -788,7 +789,7 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { if (CollectFullDiagnostics) { TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); - file << result.GetDiagnostics(); + file << diagnostics; } PrintFlameGraph(fullStats); @@ -1053,7 +1054,7 @@ void TCommandReadTable::Config(TConfig& config) { .NoArgument().SetFlag(&FromExclusive); config.Opts->AddLongOption("to-exclusive", "Don't include the right border element into response") .NoArgument().SetFlag(&ToExclusive); - + AddLegacyJsonInputFormats(config); AddOutputFormats(config, { From 71beff2a97c65ecd4717004c558c8312515c4fbd Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 22 Nov 2024 10:44:34 +0300 Subject: [PATCH 05/22] Fixes --- ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | 17 ++++++++++++++++- ydb/public/lib/ydb_cli/commands/ydb_sql.h | 1 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index b11f65a2ef3f..6173a2ca48e2 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -43,6 +44,8 @@ void TCommandSql::Config(TConfig& config) { config.Opts->AddLongOption("syntax", "Query syntax [yql, pg]") .RequiredArgument("[String]").DefaultValue("yql").StoreResult(&Syntax) .Hidden(); + config.Opts->AddLongOption("collect-diagnostics", "Collects diagnostics and saves it to file") + .StoreTrue(&CollectFullDiagnostics); AddOutputFormats(config, { EDataFormat::Pretty, @@ -146,6 +149,10 @@ int TCommandSql::RunCommand(TConfig& config) { throw TMisuseException() << "Unknow syntax option \"" << Syntax << "\""; } + if (CollectFullDiagnostics) { + settings.CollectFullDiagnostics(true); + } + if (!Parameters.empty() || InputParamStream) { // Execute query with parameters THolder paramBuilder; @@ -183,6 +190,7 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { std::optional stats; std::optional plan; std::optional ast; + TString diagnostics; { TResultSetPrinter printer(OutputFormat, &IsInterrupted); @@ -205,12 +213,14 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { plan = queryStats.GetPlan(); } } + + diagnostics = streamPart.GetDiagnostics(); } } // TResultSetPrinter destructor should be called before printing stats if (ExplainAst) { Cout << "Query AST:" << Endl << ast << Endl; - + if (IsInterrupted()) { Cerr << "" << Endl; return EXIT_FAILURE; @@ -235,6 +245,11 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { queryPlanPrinter.Print(TString{*plan}); } + if (CollectFullDiagnostics) { + TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); + file << diagnostics; + } + if (IsInterrupted()) { Cerr << "" << Endl; return EXIT_FAILURE; diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.h b/ydb/public/lib/ydb_cli/commands/ydb_sql.h index fc86a9dc3bc4..42bd3f8f94fa 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.h @@ -34,6 +34,7 @@ class TCommandSql : public TYdbCommand, public TCommandWithOutput, public TComma bool ExplainMode = false; bool ExplainAnalyzeMode = false; bool ExplainAst = false; + bool CollectDiagnostics = false; }; } From 02fc3e44981c6a21f01a461eed915ec84ee87690 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Thu, 12 Dec 2024 15:55:01 +0300 Subject: [PATCH 06/22] Fixes --- ydb/public/lib/ydb_cli/commands/ydb_sql.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.h b/ydb/public/lib/ydb_cli/commands/ydb_sql.h index 42bd3f8f94fa..60d807a75c66 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.h @@ -34,7 +34,7 @@ class TCommandSql : public TYdbCommand, public TCommandWithOutput, public TComma bool ExplainMode = false; bool ExplainAnalyzeMode = false; bool ExplainAst = false; - bool CollectDiagnostics = false; + bool CollectFullDiagnostics = false; }; } From 4ab6070d32caa7bbe49a0f1de2861cb0097ba3c1 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Mon, 16 Dec 2024 07:05:09 +0300 Subject: [PATCH 07/22] Fixes --- ydb/core/grpc_services/query/rpc_execute_query.cpp | 6 +++--- ydb/core/grpc_services/rpc_execute_data_query.cpp | 4 +--- ydb/core/kqp/common/events/query.h | 12 ++++++++---- ydb/core/kqp/common/kqp_event_impl.cpp | 8 ++------ 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/ydb/core/grpc_services/query/rpc_execute_query.cpp b/ydb/core/grpc_services/query/rpc_execute_query.cpp index 95d06d4984a8..d4466723652f 100644 --- a/ydb/core/grpc_services/query/rpc_execute_query.cpp +++ b/ydb/core/grpc_services/query/rpc_execute_query.cpp @@ -265,7 +265,8 @@ class TExecuteQueryRPC : public TActorBootstrapped { .SetUseCancelAfter(false) .SetSyntax(syntax) .SetSupportStreamTrailingResult(true) - .SetOutputChunkMaxSize(req->response_part_limit_bytes()); + .SetOutputChunkMaxSize(req->response_part_limit_bytes()) + .SetCollectFullDiagnostics(req->Getcollect_full_diagnostics()); auto ev = MakeHolder( QueryAction, @@ -281,8 +282,7 @@ class TExecuteQueryRPC : public TActorBootstrapped { cachePolicy, nullptr, // operationParams settings, - req->pool_id(), - req->Getcollect_full_diagnostics()); + req->pool_id()); ev->SetProgressStatsPeriod(TDuration::MilliSeconds(req->stats_period_ms())); diff --git a/ydb/core/grpc_services/rpc_execute_data_query.cpp b/ydb/core/grpc_services/rpc_execute_data_query.cpp index d9c6654a9926..15d0129cafd9 100644 --- a/ydb/core/grpc_services/rpc_execute_data_query.cpp +++ b/ydb/core/grpc_services/rpc_execute_data_query.cpp @@ -146,9 +146,7 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActorcollect_stats(), req->has_query_cache_policy() ? &req->query_cache_policy() : nullptr, req->has_operation_params() ? &req->operation_params() : nullptr, - NKqp::NPrivateEvents::TQueryRequestSettings(), - "", - req->Getcollect_full_diagnostics()); + NKqp::NPrivateEvents::TQueryRequestSettings().SetCollectFullDiagnostics(req->Getcollect_full_diagnostics())); ReportCostInfo_ = req->operation_params().report_cost_info() == Ydb::FeatureFlag::ENABLED; diff --git a/ydb/core/kqp/common/events/query.h b/ydb/core/kqp/common/events/query.h index a50cda93f0d1..cab2acd0e3ac 100644 --- a/ydb/core/kqp/common/events/query.h +++ b/ydb/core/kqp/common/events/query.h @@ -45,11 +45,17 @@ struct TQueryRequestSettings { return *this; } + TQueryRequestSettings& SetCollectFullDiagnostics(bool flag) { + CollectFullDiagnostics = flag; + return *this; + } + ui64 OutputChunkMaxSize = 0; bool KeepSession = false; bool UseCancelAfter = true; ::Ydb::Query::Syntax Syntax = Ydb::Query::Syntax::SYNTAX_UNSPECIFIED; bool SupportsStreamTrailingResult = false; + bool CollectFullDiagnostics = false; }; struct TEvQueryRequest: public NActors::TEventLocal { @@ -68,8 +74,7 @@ struct TEvQueryRequest: public NActors::TEventLocal collectFullDiagnostics = std::nullopt); + const TString& poolId = ""); TEvQueryRequest() { Record.MutableRequest()->SetUsePublicResponseDataFormat(true); @@ -283,7 +288,7 @@ struct TEvQueryRequest: public NActors::TEventLocal UserRequestContext; TDuration ProgressStatsPeriod; std::optional PoolConfig; - std::optional CollectFullDiagnostics = std::nullopt; }; struct TEvDataQueryStreamPart: public TEventPB collectFullDiagnostics) + const TString& poolId) : RequestCtx(ctx) , RequestActorId(requestActorId) , Database(CanonizePath(ctx->GetDatabaseName().GetOrElse(""))) @@ -36,7 +35,6 @@ TEvKqp::TEvQueryRequest::TEvQueryRequest( , QueryCachePolicy(queryCachePolicy) , HasOperationParams(operationParams) , QuerySettings(querySettings) - , CollectFullDiagnostics(collectFullDiagnostics) { if (HasOperationParams) { OperationTimeout = GetDuration(operationParams->operation_timeout()); @@ -109,9 +107,7 @@ void TEvKqp::TEvQueryRequest::PrepareRemote() const { Record.MutableRequest()->SetIsInternalCall(RequestCtx->IsInternalCall()); Record.MutableRequest()->SetOutputChunkMaxSize(QuerySettings.OutputChunkMaxSize); - if (CollectFullDiagnostics.has_value()) { - Record.MutableRequest()->SetCollectDiagnostics(CollectFullDiagnostics.value()); - } + Record.MutableRequest()->SetCollectDiagnostics(QuerySettings.CollectFullDiagnostics); RequestCtx.reset(); } From fd05673e24943d21fc538e4b6d8494f6bf14a348 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Wed, 18 Dec 2024 07:56:03 +0300 Subject: [PATCH 08/22] Fixes --- ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp | 3 ++- ydb/core/kqp/common/events/query.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp b/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp index c37dc9b06e64..c72e1ea30f02 100644 --- a/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp +++ b/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp @@ -225,7 +225,8 @@ class TStreamExecuteScanQueryRPC : public TActorBootstrappedparameters(), req->collect_stats(), nullptr, // query_cache_policy - nullptr + nullptr, + NKqp::NPrivateEvents::TQueryRequestSettings().SetCollectFullDiagnostics(req->Getcollect_full_diagnostics()) ); ev->Record.MutableRequest()->SetCollectDiagnostics(req->Getcollect_full_diagnostics()); diff --git a/ydb/core/kqp/common/events/query.h b/ydb/core/kqp/common/events/query.h index cab2acd0e3ac..8c52c47b200b 100644 --- a/ydb/core/kqp/common/events/query.h +++ b/ydb/core/kqp/common/events/query.h @@ -288,7 +288,7 @@ struct TEvQueryRequest: public NActors::TEventLocalGetCollectDiagnostics(); } ui32 CalculateSerializedSize() const override { From 560c541ef57bbc04afd92443a83f693a89214a25 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Mon, 23 Dec 2024 14:05:16 +0300 Subject: [PATCH 09/22] Fixes --- .../grpc_services/query/rpc_execute_query.cpp | 27 ++++++++++++++-- .../grpc_services/rpc_execute_data_query.cpp | 17 ++++++++-- .../rpc_stream_execute_scan_query.cpp | 31 ++++++++++++++++--- ydb/core/kqp/common/compilation/events.h | 12 +++---- ydb/core/kqp/common/events/query.h | 8 +---- ydb/core/kqp/common/kqp_event_impl.cpp | 2 -- .../kqp/compile_service/kqp_compile_actor.cpp | 12 ++++--- .../compile_service/kqp_compile_service.cpp | 14 ++++----- .../kqp/compile_service/kqp_compile_service.h | 1 + .../kqp/session_actor/kqp_query_state.cpp | 9 ++---- ydb/core/kqp/ut/olap/kqp_olap_ut.cpp | 4 +-- ydb/core/kqp/ut/query/kqp_query_ut.cpp | 2 +- ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp | 4 +-- ydb/public/api/protos/ydb_query.proto | 5 --- ydb/public/api/protos/ydb_query_stats.proto | 1 + ydb/public/api/protos/ydb_table.proto | 11 ++----- .../sdk/cpp/client/ydb_query/client.cpp | 5 --- ydb/public/sdk/cpp/client/ydb_query/client.h | 14 ++------- .../cpp/client/ydb_query/impl/exec_query.cpp | 21 +++---------- ydb/public/sdk/cpp/client/ydb_query/query.h | 1 - ydb/public/sdk/cpp/client/ydb_query/stats.cpp | 10 ++++++ ydb/public/sdk/cpp/client/ydb_query/stats.h | 1 + .../sdk/cpp/client/ydb_table/impl/readers.cpp | 7 ++--- .../client/ydb_table/impl/table_client.cpp | 1 - .../cpp/client/ydb_table/impl/table_client.h | 9 ++---- .../cpp/client/ydb_table/query_stats/stats.h | 2 +- ydb/public/sdk/cpp/client/ydb_table/table.cpp | 13 ++++---- ydb/public/sdk/cpp/client/ydb_table/table.h | 22 +++++-------- 28 files changed, 138 insertions(+), 128 deletions(-) diff --git a/ydb/core/grpc_services/query/rpc_execute_query.cpp b/ydb/core/grpc_services/query/rpc_execute_query.cpp index d4466723652f..5b4620ceb6f0 100644 --- a/ydb/core/grpc_services/query/rpc_execute_query.cpp +++ b/ydb/core/grpc_services/query/rpc_execute_query.cpp @@ -179,6 +179,25 @@ bool NeedReportAst(const Ydb::Query::ExecuteQueryRequest& req) { } } +bool NeedCollectDiagnostics(const Ydb::Query::ExecuteQueryRequest& req) { + switch (req.exec_mode()) { + case Ydb::Query::EXEC_MODE_EXPLAIN: + return true; + + case Ydb::Query::EXEC_MODE_EXECUTE: + switch (req.stats_mode()) { + case Ydb::Query::StatsMode::STATS_MODE_FULL: + case Ydb::Query::StatsMode::STATS_MODE_PROFILE: + return true; + default: + return false; + } + + default: + return false; + } +} + class TExecuteQueryRPC : public TActorBootstrapped { public: static constexpr NKikimrServices::TActivity::EType ActorActivityType() { @@ -265,8 +284,7 @@ class TExecuteQueryRPC : public TActorBootstrapped { .SetUseCancelAfter(false) .SetSyntax(syntax) .SetSupportStreamTrailingResult(true) - .SetOutputChunkMaxSize(req->response_part_limit_bytes()) - .SetCollectFullDiagnostics(req->Getcollect_full_diagnostics()); + .SetOutputChunkMaxSize(req->response_part_limit_bytes()); auto ev = MakeHolder( QueryAction, @@ -285,6 +303,7 @@ class TExecuteQueryRPC : public TActorBootstrapped { req->pool_id()); ev->SetProgressStatsPeriod(TDuration::MilliSeconds(req->stats_period_ms())); + ev->Record.MutableRequest()->SetCollectDiagnostics(NeedCollectDiagnostics(*req)); if (!ctx.Send(NKqp::MakeKqpProxyID(ctx.SelfID.NodeId()), ev.Release(), 0, 0, Span_.GetTraceId())) { NYql::TIssues issues; @@ -404,6 +423,9 @@ class TExecuteQueryRPC : public TActorBootstrapped { if (NeedReportAst(*Request_->GetProtoRequest())) { response.mutable_exec_stats()->set_query_ast(kqpResponse.GetQueryAst()); } + if (NeedCollectDiagnostics(*Request_->GetProtoRequest())) { + response.mutable_exec_stats()->set_query_diagnostics(kqpResponse.GetQueryDiagnostics()); + } } if (record.GetYdbStatus() == Ydb::StatusIds::SUCCESS) { @@ -423,7 +445,6 @@ class TExecuteQueryRPC : public TActorBootstrapped { hasTrailingMessage = true; response.mutable_tx_meta()->set_id(kqpResponse.GetTxMeta().id()); } - response.set_query_full_diagnostics(kqpResponse.GetQueryDiagnostics()); } if (hasTrailingMessage) { diff --git a/ydb/core/grpc_services/rpc_execute_data_query.cpp b/ydb/core/grpc_services/rpc_execute_data_query.cpp index 15d0129cafd9..21285699f6ff 100644 --- a/ydb/core/grpc_services/rpc_execute_data_query.cpp +++ b/ydb/core/grpc_services/rpc_execute_data_query.cpp @@ -27,6 +27,16 @@ using namespace Ydb; using namespace Ydb::Table; using namespace NKqp; +bool NeedCollectDiagnostics(const Ydb::Table::ExecuteDataQueryRequest& req) { + switch (req.collect_stats()) { + case Ydb::Table::QueryStatsCollection::STATS_COLLECTION_FULL: + case Ydb::Table::QueryStatsCollection::STATS_COLLECTION_PROFILE: + return true; + default: + return false; + } +} + using TEvExecuteDataQueryRequest = TGrpcRequestOperationCall; @@ -145,8 +155,9 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActorparameters(), req->collect_stats(), req->has_query_cache_policy() ? &req->query_cache_policy() : nullptr, - req->has_operation_params() ? &req->operation_params() : nullptr, - NKqp::NPrivateEvents::TQueryRequestSettings().SetCollectFullDiagnostics(req->Getcollect_full_diagnostics())); + req->has_operation_params() ? &req->operation_params() : nullptr); + + ev->Record.MutableRequest()->SetCollectDiagnostics(NeedCollectDiagnostics(*req)); ReportCostInfo_ = req->operation_params().report_cost_info() == Ydb::FeatureFlag::ENABLED; @@ -167,6 +178,7 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActormutable_query_stats(), from); to->mutable_query_stats()->set_query_ast(from.GetQueryAst()); + to->mutable_query_stats()->set_query_diagnostics(from.GetQueryDiagnostics()); return; } } @@ -204,7 +216,6 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActorinsert({queryParameter.GetName(), parameterType}); } } - queryResult->set_query_full_diagnostics(kqpResponse.GetQueryDiagnostics()); } catch (const std::exception& ex) { NYql::TIssues issues; issues.AddIssue(NYql::ExceptionToIssue(ex)); diff --git a/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp b/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp index c72e1ea30f02..addf6b5384d7 100644 --- a/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp +++ b/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp @@ -93,6 +93,27 @@ bool NeedReportPlan(const Ydb::Table::ExecuteScanQueryRequest& req) { } } +bool NeedCollectDiagnostics(const Ydb::Table::ExecuteScanQueryRequest& req) { + switch (req.mode()) { + case ExecuteScanQueryRequest_Mode_MODE_EXPLAIN: + return true; + + case ExecuteScanQueryRequest_Mode_MODE_EXEC: + switch (req.collect_stats()) { + case Ydb::Table::QueryStatsCollection::STATS_COLLECTION_FULL: + case Ydb::Table::QueryStatsCollection::STATS_COLLECTION_PROFILE: + return true; + default: + break; + } + + return false; + + default: + return false; + } +} + bool CheckRequest(const Ydb::Table::ExecuteScanQueryRequest& req, TParseRequestError& error) { switch (req.mode()) { @@ -225,11 +246,10 @@ class TStreamExecuteScanQueryRPC : public TActorBootstrappedparameters(), req->collect_stats(), nullptr, // query_cache_policy - nullptr, - NKqp::NPrivateEvents::TQueryRequestSettings().SetCollectFullDiagnostics(req->Getcollect_full_diagnostics()) + nullptr ); - ev->Record.MutableRequest()->SetCollectDiagnostics(req->Getcollect_full_diagnostics()); + ev->Record.MutableRequest()->SetCollectDiagnostics(NeedCollectDiagnostics(*req)); if (!ctx.Send(NKqp::MakeKqpProxyID(ctx.SelfID.NodeId()), ev.Release())) { NYql::TIssues issues; @@ -292,6 +312,7 @@ class TStreamExecuteScanQueryRPC : public TActorBootstrappedGetProtoRequest()); bool reportPlan = reportStats && NeedReportPlan(*Request_->GetProtoRequest()); + bool collectDiagnostics = NeedCollectDiagnostics(*Request_->GetProtoRequest()); if (reportStats) { if (kqpResponse.HasQueryStats()) { @@ -309,7 +330,9 @@ class TStreamExecuteScanQueryRPC : public TActorBootstrappedmutable_query_stats()->set_query_ast(kqpResponse.GetQueryAst()); } - response.mutable_result()->set_query_full_diagnostics(kqpResponse.GetQueryDiagnostics()); + if (collectDiagnostics) { + response.mutable_result()->mutable_query_stats()->set_query_diagnostics(kqpResponse.GetQueryDiagnostics()); + } Y_PROTOBUF_SUPPRESS_NODISCARD response.SerializeToString(&out); Request_->SendSerializedResult(std::move(out), record.GetYdbStatus()); diff --git a/ydb/core/kqp/common/compilation/events.h b/ydb/core/kqp/common/compilation/events.h index 9ed2d03e8735..1799c6800647 100644 --- a/ydb/core/kqp/common/compilation/events.h +++ b/ydb/core/kqp/common/compilation/events.h @@ -16,7 +16,7 @@ namespace NKikimr::NKqp::NPrivateEvents { struct TEvCompileRequest: public TEventLocal { TEvCompileRequest(const TIntrusiveConstPtr& userToken, const TString& clientAddress, const TMaybe& uid, - TMaybe&& query, bool keepInCache, bool isQueryActionPrepare, bool perStatementResult, TInstant deadline, + TMaybe&& query, bool keepInCache, NKikimrKqp::EQueryAction queryAction, bool perStatementResult, TInstant deadline, TKqpDbCountersPtr dbCounters, const TGUCSettings::TPtr& gUCSettings, const TMaybe& applicationName, std::shared_ptr> intrestedInResult, const TIntrusivePtr& userRequestContext, NLWTrace::TOrbit orbit = {}, TKqpTempTablesState::TConstPtr tempTablesState = nullptr, bool collectDiagnostics = false, TMaybe queryAst = Nothing(), @@ -26,7 +26,7 @@ struct TEvCompileRequest: public TEventLocal Uid; TMaybe Query; bool KeepInCache = false; - bool IsQueryActionPrepare = false; + NKikimrKqp::EQueryAction QueryAction; bool PerStatementResult = false; // it is allowed for local event to use absolute time (TInstant) instead of time interval (TDuration) TInstant Deadline; @@ -76,7 +76,7 @@ struct TEvCompileRequest: public TEventLocal { TEvRecompileRequest(const TIntrusiveConstPtr& userToken, const TString& clientAddress, const TString& uid, - const TMaybe& query, bool isQueryActionPrepare, TInstant deadline, + const TMaybe& query, NKikimrKqp::EQueryAction queryAction, TInstant deadline, TKqpDbCountersPtr dbCounters, const TGUCSettings::TPtr& gUCSettings, const TMaybe& applicationName, std::shared_ptr> intrestedInResult, const TIntrusivePtr& userRequestContext, NLWTrace::TOrbit orbit = {}, TKqpTempTablesState::TConstPtr tempTablesState = nullptr, TMaybe queryAst = Nothing(), @@ -85,7 +85,7 @@ struct TEvRecompileRequest: public TEventLocal Query; - bool IsQueryActionPrepare = false; + NKikimrKqp::EQueryAction QueryAction; TInstant Deadline; TKqpDbCountersPtr DbCounters; diff --git a/ydb/core/kqp/common/events/query.h b/ydb/core/kqp/common/events/query.h index 8c52c47b200b..618aa74de9f0 100644 --- a/ydb/core/kqp/common/events/query.h +++ b/ydb/core/kqp/common/events/query.h @@ -45,17 +45,11 @@ struct TQueryRequestSettings { return *this; } - TQueryRequestSettings& SetCollectFullDiagnostics(bool flag) { - CollectFullDiagnostics = flag; - return *this; - } - ui64 OutputChunkMaxSize = 0; bool KeepSession = false; bool UseCancelAfter = true; ::Ydb::Query::Syntax Syntax = Ydb::Query::Syntax::SYNTAX_UNSPECIFIED; bool SupportsStreamTrailingResult = false; - bool CollectFullDiagnostics = false; }; struct TEvQueryRequest: public NActors::TEventLocal { @@ -288,7 +282,7 @@ struct TEvQueryRequest: public NActors::TEventLocalGetCollectDiagnostics(); + return Record.MutableRequest()->GetCollectDiagnostics(); } ui32 CalculateSerializedSize() const override { diff --git a/ydb/core/kqp/common/kqp_event_impl.cpp b/ydb/core/kqp/common/kqp_event_impl.cpp index 9eac3b93a645..b6f11ca523c3 100644 --- a/ydb/core/kqp/common/kqp_event_impl.cpp +++ b/ydb/core/kqp/common/kqp_event_impl.cpp @@ -107,8 +107,6 @@ void TEvKqp::TEvQueryRequest::PrepareRemote() const { Record.MutableRequest()->SetIsInternalCall(RequestCtx->IsInternalCall()); Record.MutableRequest()->SetOutputChunkMaxSize(QuerySettings.OutputChunkMaxSize); - Record.MutableRequest()->SetCollectDiagnostics(QuerySettings.CollectFullDiagnostics); - RequestCtx.reset(); } } diff --git a/ydb/core/kqp/compile_service/kqp_compile_actor.cpp b/ydb/core/kqp/compile_service/kqp_compile_actor.cpp index 6b1813421231..51ae9e8fbd10 100644 --- a/ydb/core/kqp/compile_service/kqp_compile_actor.cpp +++ b/ydb/core/kqp/compile_service/kqp_compile_actor.cpp @@ -52,7 +52,7 @@ class TKqpCompileActor : public TActorBootstrapped { TKqpDbCountersPtr dbCounters, std::optional federatedQuerySetup, const TIntrusivePtr& userRequestContext, NWilson::TTraceId traceId, TKqpTempTablesState::TConstPtr tempTablesState, bool collectFullDiagnostics, - bool perStatementResult, + bool perStatementResult, NKikimrKqp::EQueryAction queryAction, ECompileActorAction compileAction, TMaybe queryAst, NYql::TExprContext* splitCtx, NYql::TExprNode::TPtr splitExpr) @@ -75,6 +75,7 @@ class TKqpCompileActor : public TActorBootstrapped { , CompileActorSpan(TWilsonKqp::CompileActor, std::move(traceId), "CompileActor") , TempTablesState(std::move(tempTablesState)) , CollectFullDiagnostics(collectFullDiagnostics) + , QueryAction(queryAction) , CompileAction(compileAction) , QueryAst(std::move(queryAst)) , SplitCtx(splitCtx) @@ -365,7 +366,9 @@ class TKqpCompileActor : public TActorBootstrapped { replayMessage.InsertValue("query_syntax", ToString(Config->_KqpYqlSyntaxVersion.Get().GetRef())); replayMessage.InsertValue("query_database", QueryId.Database); replayMessage.InsertValue("query_cluster", QueryId.Cluster); - replayMessage.InsertValue("query_plan", queryPlan); + if (QueryAction == NKikimrKqp::QUERY_ACTION_EXPLAIN) { + replayMessage.InsertValue("query_plan", queryPlan); + } replayMessage.InsertValue("query_type", ToString(QueryId.Settings.QueryType)); if (CollectFullDiagnostics) { @@ -613,6 +616,7 @@ class TKqpCompileActor : public TActorBootstrapped { bool CollectFullDiagnostics; bool PerStatementResult; + NKikimrKqp::EQueryAction QueryAction; ECompileActorAction CompileAction; TMaybe QueryAst; @@ -662,7 +666,7 @@ IActor* CreateKqpCompileActor(const TActorId& owner, const TKqpSettings::TConstP const TString& uid, const TKqpQueryId& query, const TIntrusiveConstPtr& userToken, const TString& clientAddress, std::optional federatedQuerySetup, TKqpDbCountersPtr dbCounters, const TGUCSettings::TPtr& gUCSettings, const TMaybe& applicationName, const TIntrusivePtr& userRequestContext, - NWilson::TTraceId traceId, TKqpTempTablesState::TConstPtr tempTablesState, + NWilson::TTraceId traceId, TKqpTempTablesState::TConstPtr tempTablesState, NKikimrKqp::EQueryAction queryAction, ECompileActorAction compileAction, TMaybe queryAst, bool collectFullDiagnostics, bool perStatementResult, NYql::TExprContext* splitCtx, NYql::TExprNode::TPtr splitExpr) { @@ -671,7 +675,7 @@ IActor* CreateKqpCompileActor(const TActorId& owner, const TKqpSettings::TConstP uid, query, userToken, clientAddress, dbCounters, federatedQuerySetup, userRequestContext, std::move(traceId), std::move(tempTablesState), collectFullDiagnostics, - perStatementResult, compileAction, std::move(queryAst), + perStatementResult, queryAction, compileAction, std::move(queryAst), splitCtx, splitExpr); } diff --git a/ydb/core/kqp/compile_service/kqp_compile_service.cpp b/ydb/core/kqp/compile_service/kqp_compile_service.cpp index 673b5a390906..fcb247e3e838 100644 --- a/ydb/core/kqp/compile_service/kqp_compile_service.cpp +++ b/ydb/core/kqp/compile_service/kqp_compile_service.cpp @@ -29,17 +29,17 @@ using namespace NYql; struct TKqpCompileSettings { - TKqpCompileSettings(bool keepInCache, bool isQueryActionPrepare, bool perStatementResult, + TKqpCompileSettings(bool keepInCache, NKikimrKqp::EQueryAction queryAction, bool perStatementResult, const TInstant& deadline, ECompileActorAction action = ECompileActorAction::COMPILE) : KeepInCache(keepInCache) - , IsQueryActionPrepare(isQueryActionPrepare) + , QueryAction(queryAction) , PerStatementResult(perStatementResult) , Deadline(deadline) , Action(action) {} bool KeepInCache; - bool IsQueryActionPrepare; + NKikimrKqp::EQueryAction QueryAction; bool PerStatementResult; TInstant Deadline; ECompileActorAction Action; @@ -461,7 +461,7 @@ class TKqpCompileService : public TActorBootstrapped { TKqpCompileSettings compileSettings( request.KeepInCache, - request.IsQueryActionPrepare, + request.QueryAction, request.PerStatementResult, request.Deadline, ev->Get()->Split @@ -529,7 +529,7 @@ class TKqpCompileService : public TActorBootstrapped { TKqpCompileSettings compileSettings( true, - request.IsQueryActionPrepare, + request.QueryAction, false, request.Deadline, ev->Get()->Split @@ -623,7 +623,7 @@ class TKqpCompileService : public TActorBootstrapped { try { if (compileResult->Status == Ydb::StatusIds::SUCCESS) { if (!hasTempTablesNameClashes) { - UpdateQueryCache(ctx, compileResult, keepInCache, compileRequest.CompileSettings.IsQueryActionPrepare, isPerStatementExecution); + UpdateQueryCache(ctx, compileResult, keepInCache, compileRequest.CompileSettings.QueryAction == NKikimrKqp::QUERY_ACTION_PREPARE, isPerStatementExecution); } if (ev->Get()->ReplayMessage && !QueryReplayBackend->IsNull()) { @@ -846,7 +846,7 @@ class TKqpCompileService : public TActorBootstrapped { void StartCompilation(TKqpCompileRequest&& request, const TActorContext& ctx) { auto compileActor = CreateKqpCompileActor(ctx.SelfID, KqpSettings, TableServiceConfig, QueryServiceConfig, ModuleResolverState, Counters, request.Uid, request.Query, request.UserToken, request.ClientAddress, FederatedQuerySetup, request.DbCounters, request.GUCSettings, request.ApplicationName, request.UserRequestContext, - request.CompileServiceSpan.GetTraceId(), request.TempTablesState, request.CompileSettings.Action, std::move(request.QueryAst), CollectDiagnostics, + request.CompileServiceSpan.GetTraceId(), request.TempTablesState, request.CompileSettings.QueryAction, request.CompileSettings.Action, std::move(request.QueryAst), CollectDiagnostics, request.CompileSettings.PerStatementResult, request.SplitCtx, request.SplitExpr); auto compileActorId = ctx.Register(compileActor, TMailboxType::HTSwap, AppData(ctx)->UserPoolId); diff --git a/ydb/core/kqp/compile_service/kqp_compile_service.h b/ydb/core/kqp/compile_service/kqp_compile_service.h index 853ba5a35910..9031019ff4a6 100644 --- a/ydb/core/kqp/compile_service/kqp_compile_service.h +++ b/ydb/core/kqp/compile_service/kqp_compile_service.h @@ -166,6 +166,7 @@ IActor* CreateKqpCompileActor(const TActorId& owner, const TKqpSettings::TConstP TKqpDbCountersPtr dbCounters, const TGUCSettings::TPtr& gUCSettings, const TMaybe& applicationName, const TIntrusivePtr& userRequestContext, NWilson::TTraceId traceId = {}, TKqpTempTablesState::TConstPtr tempTablesState = nullptr, + NKikimrKqp::EQueryAction queryAction = NKikimrKqp::QUERY_ACTION_EXECUTE, ECompileActorAction compileAction = ECompileActorAction::COMPILE, TMaybe queryAst = {}, bool collectFullDiagnostics = false, diff --git a/ydb/core/kqp/session_actor/kqp_query_state.cpp b/ydb/core/kqp/session_actor/kqp_query_state.cpp index 4c02f46b151a..1f05419948bd 100644 --- a/ydb/core/kqp/session_actor/kqp_query_state.cpp +++ b/ydb/core/kqp/session_actor/kqp_query_state.cpp @@ -300,8 +300,6 @@ std::unique_ptr TKqpQueryState::BuildCompileRequest(s compileDeadline = Min(compileDeadline, QueryDeadlines.CancelAt); } - bool isQueryActionPrepare = GetAction() == NKikimrKqp::QUERY_ACTION_PREPARE; - TMaybe statementAst; if (!Statements.empty()) { YQL_ENSURE(CurrentStatementId < Statements.size()); @@ -309,7 +307,7 @@ std::unique_ptr TKqpQueryState::BuildCompileRequest(s } return std::make_unique(UserToken, ClientAddress, uid, std::move(query), keepInCache, - isQueryActionPrepare, perStatementResult, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), + GetAction(), perStatementResult, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), UserRequestContext, std::move(Orbit), TempTablesState, GetCollectDiagnostics(), statementAst); } @@ -344,12 +342,11 @@ std::unique_ptr TKqpQueryState::BuildReCompileReque } auto compileDeadline = QueryDeadlines.TimeoutAt; - bool isQueryActionPrepare = GetAction() == NKikimrKqp::QUERY_ACTION_PREPARE; if (QueryDeadlines.CancelAt) { compileDeadline = Min(compileDeadline, QueryDeadlines.CancelAt); } - return std::make_unique(UserToken, ClientAddress, CompileResult->Uid, query, isQueryActionPrepare, + return std::make_unique(UserToken, ClientAddress, CompileResult->Uid, query, GetAction(), compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), UserRequestContext, std::move(Orbit), TempTablesState, CompileResult->QueryAst); } @@ -392,7 +389,7 @@ std::unique_ptr TKqpQueryState::BuildCompileSplittedR } return std::make_unique(UserToken, ClientAddress, uid, std::move(query), false, - false, perStatementResult, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), + NKikimrKqp::QUERY_ACTION_EXECUTE, perStatementResult, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), UserRequestContext, std::move(Orbit), TempTablesState, GetCollectDiagnostics(), statementAst, false, SplittedCtx.get(), SplittedExprs.at(NextSplittedExpr)); } diff --git a/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp b/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp index c7af14d52d8f..1b8b0bf33344 100644 --- a/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp +++ b/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp @@ -393,7 +393,7 @@ Y_UNIT_TEST_SUITE(KqpOlap) { { TStreamExecScanQuerySettings settings; - settings.CollectQueryStats(ECollectQueryStatsMode::Full); + settings.CollectQueryStats(ECollectQueryStatsMode::Basic); auto it = client.StreamExecuteScanQuery(R"( --!syntax_v1 SELECT `resource_id`, `timestamp` @@ -410,7 +410,7 @@ Y_UNIT_TEST_SUITE(KqpOlap) { { TStreamExecScanQuerySettings settings; settings.CollectQueryStats(ECollectQueryStatsMode::Full); - settings.CollectFullDiagnostics(true); + auto it = client.StreamExecuteScanQuery(R"( --!syntax_v1 SELECT `resource_id`, `timestamp` diff --git a/ydb/core/kqp/ut/query/kqp_query_ut.cpp b/ydb/core/kqp/ut/query/kqp_query_ut.cpp index ffecb6ea2ee6..1ff1c555d1ce 100644 --- a/ydb/core/kqp/ut/query/kqp_query_ut.cpp +++ b/ydb/core/kqp/ut/query/kqp_query_ut.cpp @@ -205,7 +205,7 @@ Y_UNIT_TEST_SUITE(KqpQuery) { { auto settings = TExecDataQuerySettings(); - settings.CollectFullDiagnostics(true); + settings.CollectQueryStats(ECollectQueryStatsMode::Full); auto result = session.ExecuteDataQuery(query, TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); diff --git a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp index a357617ee120..08d9ea0717be 100644 --- a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp +++ b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp @@ -656,7 +656,7 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { { TExecuteQuerySettings settings; - settings.CollectFullDiagnostics(true); + settings.StatsMode(EStatsMode::Full); auto result = db.ExecuteQuery(R"( SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; @@ -687,7 +687,7 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { { TExecuteQuerySettings settings; - settings.CollectFullDiagnostics(true); + settings.StatsMode(EStatsMode::Basic); auto result = db.ExecuteQuery(R"( SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; diff --git a/ydb/public/api/protos/ydb_query.proto b/ydb/public/api/protos/ydb_query.proto index 0427f6652e9a..b065520458cf 100644 --- a/ydb/public/api/protos/ydb_query.proto +++ b/ydb/public/api/protos/ydb_query.proto @@ -181,8 +181,6 @@ message ExecuteQueryRequest { // When query statistics are enabled (stats_mode != STATS_MODE_NONE), by default statistics will be sent only once after query execution is finished. // In case when stats_period_ms is specified and is non-zero, query statistics will be additionally sent every stats_period_ms milliseconds beginning from the start of query execution. int64 stats_period_ms = 11 [(Ydb.value) = ">= 0"]; - - bool collect_full_diagnostics = 12; } message ResultSetMeta { @@ -202,9 +200,6 @@ message ExecuteQueryResponsePart { Ydb.TableStats.QueryStats exec_stats = 5; TransactionMeta tx_meta = 6; - - // Full query diagnostics - string query_full_diagnostics = 7; } message ExecuteScriptRequest { diff --git a/ydb/public/api/protos/ydb_query_stats.proto b/ydb/public/api/protos/ydb_query_stats.proto index 300d5d9837c0..d1827b71a378 100644 --- a/ydb/public/api/protos/ydb_query_stats.proto +++ b/ydb/public/api/protos/ydb_query_stats.proto @@ -43,4 +43,5 @@ message QueryStats { string query_ast = 5; uint64 total_duration_us = 6; uint64 total_cpu_time_us = 7; + string query_diagnostics = 8; } diff --git a/ydb/public/api/protos/ydb_table.proto b/ydb/public/api/protos/ydb_table.proto index 2552f5c6987d..f01a93375430 100644 --- a/ydb/public/api/protos/ydb_table.proto +++ b/ydb/public/api/protos/ydb_table.proto @@ -926,7 +926,6 @@ message ExecuteDataQueryRequest { QueryCachePolicy query_cache_policy = 5; Ydb.Operations.OperationParams operation_params = 6; QueryStatsCollection.Mode collect_stats = 7; - bool collect_full_diagnostics = 8; } message ExecuteDataQueryResponse { @@ -969,8 +968,6 @@ message ExecuteQueryResult { QueryMeta query_meta = 3; // Query execution statistics Ydb.TableStats.QueryStats query_stats = 4; - // Full query diagnostics - string query_full_diagnostics = 5; } // Explain data query @@ -1275,9 +1272,7 @@ message ExecuteScanQueryRequest { Mode mode = 6; reserved 7; // report_progress QueryStatsCollection.Mode collect_stats = 8; - // works only in mode: MODE_EXPLAIN, - // collects additional diagnostics about query compilation, including query plan and scheme - bool collect_full_diagnostics = 9; + reserved 9; // collect_full_diagnostics } message ExecuteScanQueryPartialResponse { @@ -1293,9 +1288,7 @@ message ExecuteScanQueryPartialResult { reserved 4; // query_progress reserved 5; // query_plan Ydb.TableStats.QueryStats query_stats = 6; - // works only in mode: MODE_EXPLAIN, - // collects additional diagnostics about query compilation, including query plan and scheme - string query_full_diagnostics = 7; + reserved 7; // query_full_diagnostics } // Returns information about an external data source with a given path. diff --git a/ydb/public/sdk/cpp/client/ydb_query/client.cpp b/ydb/public/sdk/cpp/client/ydb_query/client.cpp index 8756e5f78f68..68dcb25415d9 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/client.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/client.cpp @@ -734,9 +734,4 @@ TResultSetParser TExecuteQueryResult::GetResultSetParser(size_t resultIndex) con return TResultSetParser(GetResultSet(resultIndex)); } -const TString& TExecuteQueryResult::GetDiagnostics() const { - CheckStatusOk("TExecuteQueryResult::GetDiagnostics"); - return Diagnostics_; -} - } // namespace NYdb::NQuery diff --git a/ydb/public/sdk/cpp/client/ydb_query/client.h b/ydb/public/sdk/cpp/client/ydb_query/client.h index 8d2de6be05d5..cea936c2028f 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/client.h +++ b/ydb/public/sdk/cpp/client/ydb_query/client.h @@ -216,26 +216,23 @@ class TExecuteQueryPart : public TStreamPartStatus { ui64 GetResultSetIndex() const { return ResultSetIndex_; } const TResultSet& GetResultSet() const { return *ResultSet_; } TResultSet ExtractResultSet() { return std::move(*ResultSet_); } - const TString& GetDiagnostics() const { return Diagnostics_; } const TMaybe& GetStats() const { return Stats_; } const TMaybe& GetTransaction() const { return Transaction_; } - TExecuteQueryPart(TStatus&& status, TMaybe&& queryStats, TMaybe&& tx, TString&& diagnostics) + TExecuteQueryPart(TStatus&& status, TMaybe&& queryStats, TMaybe&& tx) : TStreamPartStatus(std::move(status)) , Stats_(std::move(queryStats)) , Transaction_(std::move(tx)) - , Diagnostics_(std::move(diagnostics)) {} TExecuteQueryPart(TStatus&& status, TResultSet&& resultSet, i64 resultSetIndex, - TMaybe&& queryStats, TMaybe&& tx, TString&& diagnostics) + TMaybe&& queryStats, TMaybe&& tx) : TStreamPartStatus(std::move(status)) , ResultSet_(std::move(resultSet)) , ResultSetIndex_(resultSetIndex) , Stats_(std::move(queryStats)) , Transaction_(std::move(tx)) - , Diagnostics_(std::move(diagnostics)) {} private: @@ -243,7 +240,6 @@ class TExecuteQueryPart : public TStreamPartStatus { i64 ResultSetIndex_ = 0; TMaybe Stats_; TMaybe Transaction_; - TString Diagnostics_; }; class TExecuteQueryResult : public TStatus { @@ -256,26 +252,22 @@ class TExecuteQueryResult : public TStatus { TMaybe GetTransaction() const {return Transaction_; } - const TString& GetDiagnostics() const; - TExecuteQueryResult(TStatus&& status) : TStatus(std::move(status)) {} TExecuteQueryResult(TStatus&& status, TVector&& resultSets, - TMaybe&& stats, TMaybe&& tx, TString&& diagnostics) + TMaybe&& stats, TMaybe&& tx) : TStatus(std::move(status)) , ResultSets_(std::move(resultSets)) , Stats_(std::move(stats)) , Transaction_(std::move(tx)) - , Diagnostics_(std::move(diagnostics)) {} private: TVector ResultSets_; TMaybe Stats_; TMaybe Transaction_; - TString Diagnostics_; }; } // namespace NYdb::NQuery diff --git a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp index 37151814331d..20010fb05eca 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp @@ -70,14 +70,13 @@ class TExecuteQueryIterator::TReaderImpl { auto readCb = [self, promise](TGRpcStatus&& grpcStatus) mutable { if (!grpcStatus.Ok()) { self->Finished_ = true; - promise.SetValue({TStatus(TPlainStatus(grpcStatus, self->Endpoint_)), {}, {}, ""}); + promise.SetValue({TStatus(TPlainStatus(grpcStatus, self->Endpoint_)), {}, {}}); } else { NYql::TIssues issues; NYql::IssuesFromMessage(self->Response_.issues(), issues); EStatus clientStatus = static_cast(self->Response_.status()); TPlainStatus plainStatus{clientStatus, std::move(issues), self->Endpoint_, {}}; TStatus status{std::move(plainStatus)}; - TString diagnostics; TMaybe stats; TMaybe tx; @@ -89,19 +88,16 @@ class TExecuteQueryIterator::TReaderImpl { tx = TTransaction(self->Session_.GetRef(), self->Response_.tx_meta().id()); } - diagnostics = self->Response_.query_full_diagnostics(); - if (self->Response_.has_result_set()) { promise.SetValue({ std::move(status), TResultSet(std::move(*self->Response_.mutable_result_set())), self->Response_.result_set_index(), std::move(stats), - std::move(tx), - std::move(diagnostics) + std::move(tx) }); } else { - promise.SetValue({std::move(status), std::move(stats), std::move(tx), std::move(diagnostics)}); + promise.SetValue({std::move(status), std::move(stats), std::move(tx)}); } } }; @@ -172,12 +168,10 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { TVector issues; TVector resultProtos; TMaybe tx; - TString diagnostics; std::swap(self->Issues_, issues); std::swap(self->ResultSets_, resultProtos); std::swap(self->Tx_, tx); - std::swap(self->Diagnostics_, diagnostics); TVector resultSets; for (auto& proto : resultProtos) { @@ -188,11 +182,10 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { TStatus(EStatus::SUCCESS, NYql::TIssues(std::move(issues))), std::move(resultSets), std::move(stats), - std::move(tx), - std::move(diagnostics) + std::move(tx) )); } else { - self->Promise_.SetValue(TExecuteQueryResult(std::move(part), {}, std::move(stats), {}, {})); + self->Promise_.SetValue(TExecuteQueryResult(std::move(part), {}, std::move(stats), {})); } return; @@ -221,8 +214,6 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { self->Tx_ = tx; } - self->Diagnostics_ = part.GetDiagnostics(); - self->Next(); }); } @@ -253,8 +244,6 @@ TFuture> StreamExecuteQueryIm request.set_response_part_limit_bytes(*settings.OutputChunkMaxSize_); } - request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); - if (txControl.HasTx()) { auto requestTxControl = request.mutable_tx_control(); requestTxControl->set_commit_tx(txControl.CommitTx_); diff --git a/ydb/public/sdk/cpp/client/ydb_query/query.h b/ydb/public/sdk/cpp/client/ydb_query/query.h index a2141a7ad571..3736def97dce 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/query.h +++ b/ydb/public/sdk/cpp/client/ydb_query/query.h @@ -76,7 +76,6 @@ struct TExecuteQuerySettings : public TRequestSettings { FLUENT_SETTING_DEFAULT_DEPRECATED(EStatsMode, StatsMode, EStatsMode::None); FLUENT_SETTING_OPTIONAL_DEPRECATED(bool, ConcurrentResultSets); FLUENT_SETTING_DEPRECATED(TString, ResourcePool); - FLUENT_SETTING_DEFAULT_DEPRECATED(bool, CollectFullDiagnostics, false); }; struct TBeginTxSettings : public TRequestSettings {}; diff --git a/ydb/public/sdk/cpp/client/ydb_query/stats.cpp b/ydb/public/sdk/cpp/client/ydb_query/stats.cpp index 6b5c594f9332..54f9466c18c3 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/stats.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/stats.cpp @@ -56,6 +56,16 @@ TMaybe TExecStats::GetAst() const { return proto.query_ast(); } +TMaybe TExecStats::GetDiagnostics() const { + auto proto = Impl_->Proto; + + if (proto.query_diagnostics().empty()) { + return {}; + } + + return proto.query_diagnostics(); +} + TDuration TExecStats::GetTotalDuration() const { return TDuration::MicroSeconds(Impl_->Proto.total_duration_us()); } diff --git a/ydb/public/sdk/cpp/client/ydb_query/stats.h b/ydb/public/sdk/cpp/client/ydb_query/stats.h index a789da5a55c4..e4d2ea2a0a50 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/stats.h +++ b/ydb/public/sdk/cpp/client/ydb_query/stats.h @@ -33,6 +33,7 @@ class TExecStats { TMaybe GetPlan() const; TMaybe GetAst() const; + TMaybe GetDiagnostics() const; TDuration GetTotalDuration() const; TDuration GetTotalCpuTime() const; diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp b/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp index cc54950d2519..064992846c00 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp @@ -82,19 +82,16 @@ TAsyncScanQueryPart TScanQueryPartIterator::TReaderImpl::ReadNext(std::shared_pt TPlainStatus plainStatus{clientStatus, std::move(issues), self->Endpoint_, {}}; TStatus status{std::move(plainStatus)}; TMaybe queryStats; - TMaybe diagnostics; if (self->Response_.result().has_query_stats()) { queryStats = TQueryStats(self->Response_.result().query_stats()); } - diagnostics = self->Response_.result().query_full_diagnostics(); - if (self->Response_.result().has_result_set()) { promise.SetValue({std::move(status), - TResultSet(std::move(*self->Response_.mutable_result()->mutable_result_set())), queryStats, diagnostics}); + TResultSet(std::move(*self->Response_.mutable_result()->mutable_result_set())), queryStats}); } else { - promise.SetValue({std::move(status), queryStats, diagnostics}); + promise.SetValue({std::move(status), queryStats}); } } }; diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp index 04fa0286ce92..8442584f830b 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp @@ -1017,7 +1017,6 @@ TFuture> TT } request.set_collect_stats(GetStatsCollectionMode(settings.CollectQueryStats_)); - request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); auto promise = NewPromise>(); diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h index 7cfa440067c2..04e168d20f31 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h +++ b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.h @@ -188,8 +188,7 @@ class TTableClient::TImpl: public TClientImplCommon, public txControl.Tx_, Nothing(), false, - Nothing(), - {})); + Nothing())); } return ExecuteDataQueryInternal(session, query, txControl, params, settings, fromCache); @@ -214,7 +213,6 @@ class TTableClient::TImpl: public TClientImplCommon, public } request.set_collect_stats(GetStatsCollectionMode(settings.CollectQueryStats_)); - request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); SetQuery(query, request.mutable_query()); CollectQuerySize(query, QuerySizeHistogram); @@ -242,7 +240,6 @@ class TTableClient::TImpl: public TClientImplCommon, public TMaybe tx; TMaybe dataQuery; TMaybe queryStats; - TString diagnostics; auto queryText = GetQueryText(query); if (any) { @@ -267,8 +264,6 @@ class TTableClient::TImpl: public TClientImplCommon, public if (result.has_query_stats()) { queryStats = TQueryStats(result.query_stats()); } - - diagnostics = result.query_full_diagnostics(); } if (keepInCache && dataQuery && queryText) { @@ -276,7 +271,7 @@ class TTableClient::TImpl: public TClientImplCommon, public } TDataQueryResult dataQueryResult(TStatus(std::move(status)), - std::move(res), tx, dataQuery, fromCache, queryStats, std::move(diagnostics)); + std::move(res), tx, dataQuery, fromCache, queryStats); delete sessionPtr; tx.Clear(); diff --git a/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h b/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h index 8c8d40a52863..4b1bdc801997 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h +++ b/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h @@ -35,7 +35,7 @@ namespace NTable { enum class ECollectQueryStatsMode { None = 0, // Stats collection is disabled Basic = 1, // Aggregated stats of reads, updates and deletes per table - Full = 2, // Add per-stage execution profile and query plan on top of Basic mode + Full = 2, // Add per-stage execution profile and diagnostics on top of Basic mode Profile = 3 // Detailed execution stats including stats for individual tasks and channels }; diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.cpp b/ydb/public/sdk/cpp/client/ydb_table/table.cpp index 2d47c582cd44..be9e3925fe3c 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/table.cpp @@ -2158,15 +2158,13 @@ TTableDescription TDescribeTableResult::GetTableDescription() const { //////////////////////////////////////////////////////////////////////////////// TDataQueryResult::TDataQueryResult(TStatus&& status, TVector&& resultSets, - const TMaybe& transaction, const TMaybe& dataQuery, bool fromCache, const TMaybe &queryStats, - TString&& diagnostics) + const TMaybe& transaction, const TMaybe& dataQuery, bool fromCache, const TMaybe &queryStats) : TStatus(std::move(status)) , Transaction_(transaction) , ResultSets_(std::move(resultSets)) , DataQuery_(dataQuery) , FromCache_(fromCache) , QueryStats_(queryStats) - , Diagnostics_(std::move(diagnostics)) {} const TVector& TDataQueryResult::GetResultSets() const { @@ -2213,9 +2211,12 @@ const TString TDataQueryResult::GetQueryPlan() const { } } -const TString& TDataQueryResult::GetDiagnostics() const { - CheckStatusOk("TDataQueryResult::GetDiagnostics"); - return Diagnostics_; +const TString TDataQueryResult::GetDiagnostics() const { + if (QueryStats_.Defined()) { + return NYdb::TProtoAccessor::GetProto(*QueryStats_.Get()).query_diagnostics(); + } else { + return ""; + } } //////////////////////////////////////////////////////////////////////////////// diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.h b/ydb/public/sdk/cpp/client/ydb_table/table.h index 1be9803bbf1f..a963ec614ead 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.h +++ b/ydb/public/sdk/cpp/client/ydb_table/table.h @@ -1168,10 +1168,14 @@ struct TStreamExecScanQuerySettings : public TRequestSettings>>>>>> Fixes }; class TSession; @@ -1726,8 +1730,6 @@ struct TExecDataQuerySettings : public TOperationRequestSettings {}; @@ -2030,7 +2032,7 @@ class TDescribeTableResult : public NScheme::TDescribePathResult { class TDataQueryResult : public TStatus { public: TDataQueryResult(TStatus&& status, TVector&& resultSets, const TMaybe& transaction, - const TMaybe& dataQuery, bool fromCache, const TMaybe& queryStats, TString&& diagnostics); + const TMaybe& dataQuery, bool fromCache, const TMaybe& queryStats); const TVector& GetResultSets() const; TVector ExtractResultSets() &&; @@ -2047,7 +2049,7 @@ class TDataQueryResult : public TStatus { const TString GetQueryPlan() const; - const TString& GetDiagnostics() const; + const TString GetDiagnostics() const; private: TMaybe Transaction_; @@ -2055,7 +2057,6 @@ class TDataQueryResult : public TStatus { TMaybe DataQuery_; bool FromCache_; TMaybe QueryStats_; - TString Diagnostics_; }; class TReadTableSnapshot { @@ -2122,31 +2123,24 @@ class TScanQueryPart : public TStreamPartStatus { const TQueryStats& GetQueryStats() const { return *QueryStats_; } TQueryStats ExtractQueryStats() { return std::move(*QueryStats_); } - bool HasDiagnostics() const { return Diagnostics_.Defined(); } - const TString& GetDiagnostics() const { return *Diagnostics_; } - TString&& ExtractDiagnostics() { return std::move(*Diagnostics_); } - TScanQueryPart(TStatus&& status) : TStreamPartStatus(std::move(status)) {} - TScanQueryPart(TStatus&& status, const TMaybe& queryStats, const TMaybe& diagnostics) + TScanQueryPart(TStatus&& status, const TMaybe& queryStats) : TStreamPartStatus(std::move(status)) , QueryStats_(queryStats) - , Diagnostics_(diagnostics) {} - TScanQueryPart(TStatus&& status, TResultSet&& resultSet, const TMaybe& queryStats, const TMaybe& diagnostics) + TScanQueryPart(TStatus&& status, TResultSet&& resultSet, const TMaybe& queryStats) : TStreamPartStatus(std::move(status)) , ResultSet_(std::move(resultSet)) , QueryStats_(queryStats) - , Diagnostics_(diagnostics) {} private: TMaybe ResultSet_; TMaybe QueryStats_; - TMaybe Diagnostics_; }; using TAsyncScanQueryPart = NThreading::TFuture; From a2a9a5a6ef2e9dd29b22b96143658c3eb07d0255 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 27 Dec 2024 13:35:04 +0300 Subject: [PATCH 10/22] Fixes --- ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp | 88 ++++++++++++++++++- .../ydb_cli/commands/ydb_service_table.cpp | 39 +------- .../lib/ydb_cli/commands/ydb_service_table.h | 1 - ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | 45 +++++++--- ydb/public/lib/ydb_cli/commands/ydb_sql.h | 2 +- ydb/public/sdk/cpp/client/ydb_query/stats.cpp | 1 + 6 files changed, 120 insertions(+), 56 deletions(-) diff --git a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp index 08d9ea0717be..69790f46aa29 100644 --- a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp +++ b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp @@ -663,10 +663,11 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); - UNIT_ASSERT_C(!result.GetDiagnostics().empty(), "Query result diagnostics is empty"); + auto& stats = NYdb::TProtoAccessor::GetProto(*result.GetStats()); + UNIT_ASSERT_C(!stats.query_diagnostics().empty(), "Query result diagnostics is empty"); TStringStream in; - in << result.GetDiagnostics(); + in << stats.query_diagnostics(); NJson::TJsonValue value; ReadJsonTree(&in, &value); @@ -681,7 +682,7 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_plan"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(!value.Has("query_plan"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Diagnostics"); } @@ -695,7 +696,86 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); - UNIT_ASSERT_C(result.GetDiagnostics().empty(), "Query result diagnostics should be empty, but it's not"); + auto& stats = NYdb::TProtoAccessor::GetProto(*result.GetStats()); + UNIT_ASSERT_C(stats.query_diagnostics().empty(), "Query result diagnostics should be empty, but it's not"); + } + } + + Y_UNIT_TEST(StreamExecuteCollectFullDiagnostics) { + auto kikimr = DefaultKikimrRunner(); + auto db = kikimr.GetQueryClient(); + + { + TExecuteQuerySettings settings; + settings.StatsMode(EStatsMode::Full); + + auto it = db.StreamExecuteQuery(R"( + SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; + )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(it.GetStatus(), EStatus::SUCCESS, it.GetIssues().ToString()); + + TString statsString; + for (;;) { + auto streamPart = it.ReadNext().GetValueSync(); + if (!streamPart.IsSuccess()) { + UNIT_ASSERT_C(streamPart.EOS(), streamPart.GetIssues().ToString()); + break; + } + + const auto& execStats = streamPart.GetStats(); + if (execStats.Defined()) { + auto& stats = NYdb::TProtoAccessor::GetProto(*execStats); + statsString = stats.query_diagnostics(); + } + } + + UNIT_ASSERT_C(!statsString.empty(), "Query result diagnostics is empty"); + + TStringStream in; + in << statsString; + NJson::TJsonValue value; + ReadJsonTree(&in, &value); + + UNIT_ASSERT_C(value.IsMap(), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("version"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_text"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Diagnostics: table_metadata type should be an array"); + UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(!value.Has("query_plan"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Diagnostics"); + } + + { + TExecuteQuerySettings settings; + settings.StatsMode(EStatsMode::Basic); + + auto it = db.StreamExecuteQuery(R"( + SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; + )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(it.GetStatus(), EStatus::SUCCESS, it.GetIssues().ToString()); + + TString statsString; + for (;;) { + auto streamPart = it.ReadNext().GetValueSync(); + if (!streamPart.IsSuccess()) { + UNIT_ASSERT_C(streamPart.EOS(), streamPart.GetIssues().ToString()); + break; + } + + const auto& execStats = streamPart.GetStats(); + if (execStats.Defined()) { + auto& stats = NYdb::TProtoAccessor::GetProto(*execStats); + statsString = stats.query_diagnostics(); + } + } + + UNIT_ASSERT_C(statsString.empty(), "Query result diagnostics should be empty, but it's not"); } } diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index 7eca82e02576..a56cf093034a 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -365,8 +365,6 @@ void TCommandExecuteQuery::Config(TConfig& config) { config.Opts->AddLongOption('q', "query", "Text of query to execute").RequiredArgument("[String]").StoreResult(&Query); config.Opts->AddLongOption('f', "file", "Path to file with query text to execute") .RequiredArgument("PATH").StoreResult(&QueryFile); - config.Opts->AddLongOption("collect-diagnostics", "Collects diagnostics and saves it to file") - .StoreTrue(&CollectFullDiagnostics); AddOutputFormats(config, { EDataFormat::Pretty, @@ -434,9 +432,6 @@ int TCommandExecuteQuery::ExecuteDataQuery(TConfig& config) { NTable::TExecDataQuerySettings settings; settings.KeepInQueryCache(true); settings.CollectQueryStats(ParseQueryStatsModeOrThrow(CollectStatsMode, defaultStatsMode)); - if (CollectFullDiagnostics) { - settings.CollectFullDiagnostics(true); - } NTable::TTxSettings txSettings; if (TxMode) { @@ -521,11 +516,6 @@ void TCommandExecuteQuery::PrintDataQueryResponse(NTable::TDataQueryResult& resu { Cout << Endl << "Flame graph is available for full or profile stats only" << Endl; } - if (CollectFullDiagnostics) - { - TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); - file << result.GetDiagnostics(); - } } int TCommandExecuteQuery::ExecuteSchemeQuery(TConfig& config) { @@ -558,7 +548,7 @@ namespace { NQuery::TExecuteQuerySettings>; template - auto GetSettings(const TString& collectStatsMode, const bool basicStats, std::optional timeout, bool collectFullDiagnostics) { + auto GetSettings(const TString& collectStatsMode, const bool basicStats, std::optional timeout) { if constexpr (std::is_same_v) { const auto defaultStatsMode = basicStats ? NTable::ECollectQueryStatsMode::Basic @@ -568,9 +558,6 @@ namespace { if (timeout.has_value()) { settings.ClientTimeout(*timeout); } - if (collectFullDiagnostics) { - settings.CollectFullDiagnostics(true); - } return settings; } else if constexpr (std::is_same_v) { const auto defaultStatsMode = basicStats @@ -581,9 +568,6 @@ namespace { if (timeout.has_value()) { settings.ClientTimeout(*timeout); } - if (collectFullDiagnostics) { - settings.CollectFullDiagnostics(true); - } return settings; } Y_UNREACHABLE(); @@ -690,7 +674,7 @@ int TCommandExecuteQuery::ExecuteQueryImpl(TConfig& config) { if (OperationTimeout) { optTimeout = TDuration::MilliSeconds(FromString(OperationTimeout)); } - const auto settings = GetSettings(CollectStatsMode, BasicStats, optTimeout, CollectFullDiagnostics); + const auto settings = GetSettings(CollectStatsMode, BasicStats, optTimeout); TAsyncPartIterator asyncResult; SetInterruptHandlers(); @@ -770,8 +754,6 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { fullStats = queryStats.GetPlan(); } } - - diagnostics = streamPart.GetDiagnostics(); } } // TResultSetPrinter destructor should be called before printing stats @@ -786,12 +768,6 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { queryPlanPrinter.Print(TString{*fullStats}); } - if (CollectFullDiagnostics) - { - TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); - file << diagnostics; - } - PrintFlameGraph(fullStats); if (IsInterrupted()) { @@ -897,10 +873,6 @@ int TCommandExplain::Run(TConfig& config) { settings.Explain(true); } - if (CollectFullDiagnostics) { - settings.CollectFullDiagnostics(true); - } - auto result = client.StreamExecuteScanQuery(Query, settings).GetValueSync(); NStatusHelpers::ThrowOnErrorOrPrintIssues(result); @@ -917,13 +889,6 @@ int TCommandExplain::Run(TConfig& config) { planJson = proto.query_plan(); ast = proto.query_ast(); } - if (tablePart.HasDiagnostics()) { - diagnostics = tablePart.ExtractDiagnostics(); - } - } - - if (CollectFullDiagnostics) { - SaveDiagnosticsToFile(diagnostics); } if (IsInterrupted()) { diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.h b/ydb/public/lib/ydb_cli/commands/ydb_service_table.h index 7fe5754ebf52..041543abb3ef 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.h @@ -123,7 +123,6 @@ class TCommandExecuteQuery : public TTableCommand, TCommandQueryBase, TCommandWi TString TxMode; TString QueryType; bool BasicStats = false; - bool CollectFullDiagnostics = false; }; class TCommandExplain : public TTableCommand, public TCommandWithOutput, TCommandQueryBase, TInterruptibleCommand { diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index 6173a2ca48e2..d98c9d6a999a 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp @@ -1,6 +1,8 @@ #include "ydb_sql.h" #include +#include +#include #include #include #include @@ -9,7 +11,6 @@ #include #include #include -#include #include #include @@ -41,11 +42,11 @@ void TCommandSql::Config(TConfig& config) { .StoreTrue(&ExplainAnalyzeMode); config.Opts->AddLongOption("stats", "Execution statistics collection mode [none, basic, full, profile]") .RequiredArgument("[String]").StoreResult(&CollectStatsMode); + config.Opts->AddLongOption("diagnostics-file", "Path to file where the diagnostics will be saved.") + .RequiredArgument("[String]").StoreResult(&DiagnosticsFile); config.Opts->AddLongOption("syntax", "Query syntax [yql, pg]") .RequiredArgument("[String]").DefaultValue("yql").StoreResult(&Syntax) .Hidden(); - config.Opts->AddLongOption("collect-diagnostics", "Collects diagnostics and saves it to file") - .StoreTrue(&CollectFullDiagnostics); AddOutputFormats(config, { EDataFormat::Pretty, @@ -149,10 +150,6 @@ int TCommandSql::RunCommand(TConfig& config) { throw TMisuseException() << "Unknow syntax option \"" << Syntax << "\""; } - if (CollectFullDiagnostics) { - settings.CollectFullDiagnostics(true); - } - if (!Parameters.empty() || InputParamStream) { // Execute query with parameters THolder paramBuilder; @@ -190,7 +187,7 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { std::optional stats; std::optional plan; std::optional ast; - TString diagnostics; + TMaybe meta; { TResultSetPrinter printer(OutputFormat, &IsInterrupted); @@ -212,9 +209,8 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { if (queryStats.GetPlan()) { plan = queryStats.GetPlan(); } + diagnostics = queryStats.GetDiagnostics(); } - - diagnostics = streamPart.GetDiagnostics(); } } // TResultSetPrinter destructor should be called before printing stats @@ -232,6 +228,10 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { Cout << Endl << "Statistics:" << Endl << *stats; } + if (diagnostics) { + Cout << Endl << "Diagnostics:" << Endl << NJson::PrettifyJson(*diagnostics, true) << Endl;; + } + if (plan) { if (!ExplainMode && !ExplainAnalyzeMode && (OutputFormat == EDataFormat::Default || OutputFormat == EDataFormat::Pretty)) { @@ -245,9 +245,28 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { queryPlanPrinter.Print(TString{*plan}); } - if (CollectFullDiagnostics) { - TFileOutput file(TStringBuilder() << "diagnostics_" << TGUID::Create().AsGuidString() << ".txt"); - file << diagnostics; + if (!DiagnosticsFile.empty()) { + TFileOutput file(DiagnosticsFile); + + NJson::TJsonValue diagnosticsJson(NJson::JSON_MAP); + + if (stats) { + diagnosticsJson.InsertValue("stats", *stats); + } + if (ast) { + diagnosticsJson.InsertValue("ast", *ast); + } + if (plan) { + NJson::TJsonValue planJson; + NJson::ReadJsonTree(*plan, &planJson, true); + diagnosticsJson.InsertValue("plan", planJson); + } + if (diagnostics) { + NJson::TJsonValue metaJson; + NJson::ReadJsonTree(*diagnostics, &metaJson, true); + diagnosticsJson.InsertValue("meta", metaJson); + } + file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), false); } if (IsInterrupted()) { diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.h b/ydb/public/lib/ydb_cli/commands/ydb_sql.h index 60d807a75c66..bfa617ae66cb 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.h @@ -28,13 +28,13 @@ class TCommandSql : public TYdbCommand, public TCommandWithOutput, public TComma int PrintResponse(NQuery::TExecuteQueryIterator& result); TString CollectStatsMode; + TString DiagnosticsFile; TString Query; TString QueryFile; TString Syntax; bool ExplainMode = false; bool ExplainAnalyzeMode = false; bool ExplainAst = false; - bool CollectFullDiagnostics = false; }; } diff --git a/ydb/public/sdk/cpp/client/ydb_query/stats.cpp b/ydb/public/sdk/cpp/client/ydb_query/stats.cpp index 54f9466c18c3..f4577d6b6761 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/stats.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/stats.cpp @@ -29,6 +29,7 @@ TString TExecStats::ToString(bool withPlan) const { if (!withPlan) { proto.clear_query_plan(); proto.clear_query_ast(); + proto.clear_query_diagnostics(); } TString res; From 526949b4e34867e5435d07cd437e42212e821c5e Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Thu, 13 Feb 2025 23:26:54 +0300 Subject: [PATCH 11/22] Fixes --- .../grpc_services/query/rpc_execute_query.cpp | 2 +- .../grpc_services/rpc_execute_data_query.cpp | 6 +- .../rpc_stream_execute_scan_query.cpp | 2 +- .../kqp/compile_service/kqp_compile_actor.cpp | 2 +- .../kqp/ut/olap/helpers/query_executor.cpp | 16 ++-- ydb/core/kqp/ut/olap/kqp_olap_ut.cpp | 41 +++++---- ydb/core/kqp/ut/query/kqp_query_ut.cpp | 33 ++++---- ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp | 72 ++++++++-------- ydb/public/api/protos/ydb_query_stats.proto | 4 +- ydb/public/api/protos/ydb_table.proto | 8 +- .../ydb_cli/commands/ydb_service_table.cpp | 83 +++++++++++++++++-- .../lib/ydb_cli/commands/ydb_service_table.h | 1 + ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | 15 ++-- .../cpp/client/ydb_query/impl/exec_query.cpp | 1 - ydb/public/sdk/cpp/client/ydb_query/stats.cpp | 8 +- ydb/public/sdk/cpp/client/ydb_query/stats.h | 2 +- ydb/public/sdk/cpp/client/ydb_table/table.cpp | 4 +- ydb/public/sdk/cpp/client/ydb_table/table.h | 2 +- 18 files changed, 190 insertions(+), 112 deletions(-) diff --git a/ydb/core/grpc_services/query/rpc_execute_query.cpp b/ydb/core/grpc_services/query/rpc_execute_query.cpp index 5b4620ceb6f0..8e0fe7252525 100644 --- a/ydb/core/grpc_services/query/rpc_execute_query.cpp +++ b/ydb/core/grpc_services/query/rpc_execute_query.cpp @@ -424,7 +424,7 @@ class TExecuteQueryRPC : public TActorBootstrapped { response.mutable_exec_stats()->set_query_ast(kqpResponse.GetQueryAst()); } if (NeedCollectDiagnostics(*Request_->GetProtoRequest())) { - response.mutable_exec_stats()->set_query_diagnostics(kqpResponse.GetQueryDiagnostics()); + response.mutable_exec_stats()->set_query_meta(kqpResponse.GetQueryDiagnostics()); } } diff --git a/ydb/core/grpc_services/rpc_execute_data_query.cpp b/ydb/core/grpc_services/rpc_execute_data_query.cpp index 21285699f6ff..eb98c29674aa 100644 --- a/ydb/core/grpc_services/rpc_execute_data_query.cpp +++ b/ydb/core/grpc_services/rpc_execute_data_query.cpp @@ -156,7 +156,7 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActorcollect_stats(), req->has_query_cache_policy() ? &req->query_cache_policy() : nullptr, req->has_operation_params() ? &req->operation_params() : nullptr); - + ev->Record.MutableRequest()->SetCollectDiagnostics(NeedCollectDiagnostics(*req)); ReportCostInfo_ = req->operation_params().report_cost_info() == Ydb::FeatureFlag::ENABLED; @@ -178,7 +178,9 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActormutable_query_stats(), from); to->mutable_query_stats()->set_query_ast(from.GetQueryAst()); - to->mutable_query_stats()->set_query_diagnostics(from.GetQueryDiagnostics()); + if (from.HasQueryDiagnostics()) { + to->mutable_query_stats()->set_query_meta(from.GetQueryDiagnostics()); + } return; } } diff --git a/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp b/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp index addf6b5384d7..6d2a6713be83 100644 --- a/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp +++ b/ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp @@ -331,7 +331,7 @@ class TStreamExecuteScanQueryRPC : public TActorBootstrappedmutable_query_stats()->set_query_diagnostics(kqpResponse.GetQueryDiagnostics()); + response.mutable_result()->mutable_query_stats()->set_query_meta(kqpResponse.GetQueryDiagnostics()); } Y_PROTOBUF_SUPPRESS_NODISCARD response.SerializeToString(&out); diff --git a/ydb/core/kqp/compile_service/kqp_compile_actor.cpp b/ydb/core/kqp/compile_service/kqp_compile_actor.cpp index 51ae9e8fbd10..d02b1af25003 100644 --- a/ydb/core/kqp/compile_service/kqp_compile_actor.cpp +++ b/ydb/core/kqp/compile_service/kqp_compile_actor.cpp @@ -354,7 +354,6 @@ class TKqpCompileActor : public TActorBootstrapped { replayMessage.InsertValue("query_id", Uid); replayMessage.InsertValue("version", "1.0"); - replayMessage.InsertValue("query_text", EscapeC(QueryId.Text)); NJson::TJsonValue queryParameterTypes(NJson::JSON_MAP); if (QueryId.QueryParameterTypes) { for (const auto& [paramName, paramType] : *QueryId.QueryParameterTypes) { @@ -383,6 +382,7 @@ class TKqpCompileActor : public TActorBootstrapped { ReplayMessageUserView = NJson::WriteJson(replayMessage, /*formatOutput*/ false); } + replayMessage.InsertValue("query_text", EscapeC(QueryId.Text)); replayMessage.InsertValue("table_metadata", TString(NJson::WriteJson(tablesMeta, false))); replayMessage.InsertValue("table_meta_serialization_type", EMetaSerializationType::EncodedProto); diff --git a/ydb/core/kqp/ut/olap/helpers/query_executor.cpp b/ydb/core/kqp/ut/olap/helpers/query_executor.cpp index 7dbf61dd2f8f..648b5db6b95a 100644 --- a/ydb/core/kqp/ut/olap/helpers/query_executor.cpp +++ b/ydb/core/kqp/ut/olap/helpers/query_executor.cpp @@ -5,13 +5,13 @@ namespace NKikimr::NKqp { -TVector> CollectRows(NYdb::NTable::TScanQueryPartIterator& it, NJson::TJsonValue* statInfo /*= nullptr*/, NJson::TJsonValue* diagnostics /*= nullptr*/) { +TVector> CollectRows(NYdb::NTable::TScanQueryPartIterator& it, NJson::TJsonValue* statInfo /*= nullptr*/, NJson::TJsonValue* meta /*= nullptr*/) { TVector> rows; if (statInfo) { *statInfo = NJson::JSON_NULL; } - if (diagnostics) { - *diagnostics = NJson::JSON_NULL; + if (meta) { + *meta = NJson::JSON_NULL; } for (;;) { auto streamPart = it.ReadNext().GetValueSync(); @@ -28,12 +28,10 @@ TVector> CollectRows(NYdb::NTable::TScanQueryPar if (plan && statInfo) { UNIT_ASSERT(NJson::ReadJsonFastTree(*plan, statInfo)); } - } - if (streamPart.HasDiagnostics()) { - auto diagnosticsString = TString{streamPart.GetDiagnostics()}; - if (!diagnosticsString.empty() && diagnostics) { - UNIT_ASSERT(NJson::ReadJsonFastTree(diagnosticsString, diagnostics)); + auto metaString = streamPart.GetQueryStats().GetMeta(); + if (metaString && !metaString->empty() && meta) { + UNIT_ASSERT(NJson::ReadJsonFastTree(*metaString, meta)); } } @@ -70,4 +68,4 @@ TVector> ExecuteScanQuery(NYdb::NTable::TTableCl return rows; } -} \ No newline at end of file +} diff --git a/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp b/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp index 1b8b0bf33344..c31081525192 100644 --- a/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp +++ b/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp @@ -380,7 +380,7 @@ Y_UNIT_TEST_SUITE(KqpOlap) { } } - Y_UNIT_TEST(SimpleQueryOlapDiagnostics) { + Y_UNIT_TEST(SimpleQueryOlapMeta) { auto settings = TKikimrSettings() .SetWithSampleTables(false); TKikimrRunner kikimr(settings); @@ -402,9 +402,9 @@ Y_UNIT_TEST_SUITE(KqpOlap) { )", settings).GetValueSync(); UNIT_ASSERT_C(it.IsSuccess(), it.GetIssues().ToString()); - NJson::TJsonValue jsonDiagnostics; - CollectRows(it, nullptr, &jsonDiagnostics); - UNIT_ASSERT_C(!jsonDiagnostics.IsDefined(), "Query result diagnostics should be empty, but it's not"); + NJson::TJsonValue jsonMeta; + CollectRows(it, nullptr, &jsonMeta); + UNIT_ASSERT_C(!jsonMeta.IsDefined(), "Query result meta should be empty, but it's not"); } { @@ -419,22 +419,21 @@ Y_UNIT_TEST_SUITE(KqpOlap) { )", settings).GetValueSync(); UNIT_ASSERT_C(it.IsSuccess(), it.GetIssues().ToString()); - NJson::TJsonValue jsonDiagnostics; - CollectRows(it, nullptr, &jsonDiagnostics); - UNIT_ASSERT(!jsonDiagnostics.IsNull()); - - UNIT_ASSERT_C(jsonDiagnostics.IsMap(), "Incorrect Diagnostics"); - UNIT_ASSERT_C(jsonDiagnostics.Has("query_id"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(jsonDiagnostics.Has("version"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(jsonDiagnostics.Has("query_text"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(jsonDiagnostics.Has("query_parameter_types"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(jsonDiagnostics.Has("table_metadata"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(jsonDiagnostics.Has("created_at"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(jsonDiagnostics.Has("query_syntax"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(jsonDiagnostics.Has("query_database"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(jsonDiagnostics.Has("query_cluster"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(jsonDiagnostics.Has("query_plan"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(jsonDiagnostics.Has("query_type"), "Incorrect Diagnostics"); + NJson::TJsonValue jsonMeta; + CollectRows(it, nullptr, &jsonMeta); + UNIT_ASSERT(!jsonMeta.IsNull()); + + UNIT_ASSERT_C(jsonMeta.IsMap(), "Incorrect Meta"); + UNIT_ASSERT_C(jsonMeta.Has("query_id"), "Incorrect Meta"); + UNIT_ASSERT_C(jsonMeta.Has("version"), "Incorrect Meta"); + UNIT_ASSERT_C(jsonMeta.Has("query_parameter_types"), "Incorrect Meta"); + UNIT_ASSERT_C(jsonMeta.Has("table_metadata"), "Incorrect Meta"); + UNIT_ASSERT_C(jsonMeta.Has("created_at"), "Incorrect Meta"); + UNIT_ASSERT_C(jsonMeta.Has("query_syntax"), "Incorrect Meta"); + UNIT_ASSERT_C(jsonMeta.Has("query_database"), "Incorrect Meta"); + UNIT_ASSERT_C(jsonMeta.Has("query_cluster"), "Incorrect Meta"); + UNIT_ASSERT_C(!jsonMeta.Has("query_plan"), "Incorrect Meta"); + UNIT_ASSERT_C(jsonMeta.Has("query_type"), "Incorrect Meta"); } } @@ -2785,7 +2784,7 @@ Y_UNIT_TEST_SUITE(KqpOlap) { auto alterQuery = TStringBuilder() << R"(ALTER OBJECT `/Root/olapStore` (TYPE TABLESTORE) SET (ACTION=UPSERT_OPTIONS, `COMPACTION_PLANNER.CLASS_NAME`=`lc-buckets`, `COMPACTION_PLANNER.FEATURES`=` - {"levels" : [{"class_name" : "Zero", "portions_live_duration" : "180s", "expected_blobs_size" : 2048000}, + {"levels" : [{"class_name" : "Zero", "portions_live_duration" : "180s", "expected_blobs_size" : 2048000}, {"class_name" : "Zero", "expected_blobs_size" : 2048000}, {"class_name" : "Zero"}]}`); )"; auto session = tableClient.CreateSession().GetValueSync().GetSession(); diff --git a/ydb/core/kqp/ut/query/kqp_query_ut.cpp b/ydb/core/kqp/ut/query/kqp_query_ut.cpp index 1ff1c555d1ce..5cccc5c3221b 100644 --- a/ydb/core/kqp/ut/query/kqp_query_ut.cpp +++ b/ydb/core/kqp/ut/query/kqp_query_ut.cpp @@ -179,7 +179,7 @@ Y_UNIT_TEST_SUITE(KqpQuery) { UNIT_ASSERT_VALUES_EQUAL(counters.RecompileRequestGet()->Val(), 1); } - Y_UNIT_TEST(ExecuteDataQueryCollectFullDiagnostics) { + Y_UNIT_TEST(ExecuteDataQueryCollectMeta) { auto setting = NKikimrKqp::TKqpSetting(); auto serverSettings = TKikimrSettings() .SetKqpSettings({setting}); @@ -211,26 +211,25 @@ Y_UNIT_TEST_SUITE(KqpQuery) { UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); - UNIT_ASSERT_C(!result.GetDiagnostics().empty(), "Query result diagnostics is empty"); + UNIT_ASSERT_C(!result.GetMeta().empty(), "Query result meta is empty"); TStringStream in; - in << result.GetDiagnostics(); + in << result.GetMeta(); NJson::TJsonValue value; ReadJsonTree(&in, &value); - UNIT_ASSERT_C(value.IsMap(), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("version"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_text"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Diagnostics: table_metadata type should be an array"); - UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_plan"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.IsMap(), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("version"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Meta"); + UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Meta: table_metadata type should be an array"); + UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Meta"); + UNIT_ASSERT_C(!value.Has("query_plan"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Meta"); } { @@ -240,7 +239,7 @@ Y_UNIT_TEST_SUITE(KqpQuery) { UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); - UNIT_ASSERT_C(result.GetDiagnostics().empty(), "Query result diagnostics should be empty, but it's not"); + UNIT_ASSERT_C(result.GetMeta().empty(), "Query result meta should be empty, but it's not"); } } } diff --git a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp index 69790f46aa29..b6a461469135 100644 --- a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp +++ b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp @@ -650,7 +650,7 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { } } - Y_UNIT_TEST(ExecuteCollectFullDiagnostics) { + Y_UNIT_TEST(ExecuteCollectMeta) { auto kikimr = DefaultKikimrRunner(); auto db = kikimr.GetQueryClient(); @@ -661,29 +661,28 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { auto result = db.ExecuteQuery(R"( SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); - + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); auto& stats = NYdb::TProtoAccessor::GetProto(*result.GetStats()); - UNIT_ASSERT_C(!stats.query_diagnostics().empty(), "Query result diagnostics is empty"); + UNIT_ASSERT_C(!stats.query_meta().empty(), "Query result meta is empty"); TStringStream in; - in << stats.query_diagnostics(); + in << stats.query_meta(); NJson::TJsonValue value; ReadJsonTree(&in, &value); - UNIT_ASSERT_C(value.IsMap(), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("version"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_text"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Diagnostics: table_metadata type should be an array"); - UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(!value.Has("query_plan"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.IsMap(), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("version"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Meta"); + UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Meta: table_metadata type should be an array"); + UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Meta"); + UNIT_ASSERT_C(!value.Has("query_plan"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Meta"); } { @@ -693,15 +692,15 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { auto result = db.ExecuteQuery(R"( SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); - + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); auto& stats = NYdb::TProtoAccessor::GetProto(*result.GetStats()); - UNIT_ASSERT_C(stats.query_diagnostics().empty(), "Query result diagnostics should be empty, but it's not"); + UNIT_ASSERT_C(stats.query_meta().empty(), "Query result Meta should be empty, but it's not"); } } - Y_UNIT_TEST(StreamExecuteCollectFullDiagnostics) { + Y_UNIT_TEST(StreamExecuteCollectMeta) { auto kikimr = DefaultKikimrRunner(); auto db = kikimr.GetQueryClient(); @@ -725,30 +724,29 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { const auto& execStats = streamPart.GetStats(); if (execStats.Defined()) { auto& stats = NYdb::TProtoAccessor::GetProto(*execStats); - statsString = stats.query_diagnostics(); + statsString = stats.query_meta(); } } - UNIT_ASSERT_C(!statsString.empty(), "Query result diagnostics is empty"); + UNIT_ASSERT_C(!statsString.empty(), "Query result meta is empty"); TStringStream in; in << statsString; NJson::TJsonValue value; ReadJsonTree(&in, &value); - UNIT_ASSERT_C(value.IsMap(), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("version"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_text"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Diagnostics: table_metadata type should be an array"); - UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(!value.Has("query_plan"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(value.IsMap(), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("version"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Meta"); + UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Meta: table_metadata type should be an array"); + UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Meta"); + UNIT_ASSERT_C(!value.Has("query_plan"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Meta"); } { @@ -771,11 +769,11 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { const auto& execStats = streamPart.GetStats(); if (execStats.Defined()) { auto& stats = NYdb::TProtoAccessor::GetProto(*execStats); - statsString = stats.query_diagnostics(); + statsString = stats.query_meta(); } } - UNIT_ASSERT_C(statsString.empty(), "Query result diagnostics should be empty, but it's not"); + UNIT_ASSERT_C(statsString.empty(), "Query result meta should be empty, but it's not"); } } diff --git a/ydb/public/api/protos/ydb_query_stats.proto b/ydb/public/api/protos/ydb_query_stats.proto index d1827b71a378..34f4f49bdb1c 100644 --- a/ydb/public/api/protos/ydb_query_stats.proto +++ b/ydb/public/api/protos/ydb_query_stats.proto @@ -43,5 +43,7 @@ message QueryStats { string query_ast = 5; uint64 total_duration_us = 6; uint64 total_cpu_time_us = 7; - string query_diagnostics = 8; + // will be filled only in MODE_EXPLAIN or in MODE_EXEC with QueryStatsCollection.Mode >= STATS_COLLECTION_FULL, + // collects additional meta about query compilation, including table metadata + string query_meta = 8; } diff --git a/ydb/public/api/protos/ydb_table.proto b/ydb/public/api/protos/ydb_table.proto index f01a93375430..3d69ae628f05 100644 --- a/ydb/public/api/protos/ydb_table.proto +++ b/ydb/public/api/protos/ydb_table.proto @@ -1272,7 +1272,9 @@ message ExecuteScanQueryRequest { Mode mode = 6; reserved 7; // report_progress QueryStatsCollection.Mode collect_stats = 8; - reserved 9; // collect_full_diagnostics + // works only in mode: MODE_EXPLAIN, + // collects additional diagnostics about query compilation, including query plan and scheme + bool collect_full_diagnostics = 9 [deprecated=true]; } message ExecuteScanQueryPartialResponse { @@ -1288,7 +1290,9 @@ message ExecuteScanQueryPartialResult { reserved 4; // query_progress reserved 5; // query_plan Ydb.TableStats.QueryStats query_stats = 6; - reserved 7; // query_full_diagnostics + // works only in mode: MODE_EXPLAIN, + // collects additional diagnostics about query compilation, including query plan and scheme + string query_full_diagnostics = 7 [deprecated = true]; } // Returns information about an external data source with a given path. diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index a56cf093034a..9d28f4251673 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -9,8 +9,11 @@ #include #include +#include + #include +#include #include #include #include @@ -365,6 +368,8 @@ void TCommandExecuteQuery::Config(TConfig& config) { config.Opts->AddLongOption('q', "query", "Text of query to execute").RequiredArgument("[String]").StoreResult(&Query); config.Opts->AddLongOption('f', "file", "Path to file with query text to execute") .RequiredArgument("PATH").StoreResult(&QueryFile); + config.Opts->AddLongOption("diagnostics-file", "Path to file where the diagnostics will be saved.") + .RequiredArgument("[String]").StoreResult(&DiagnosticsFile); AddOutputFormats(config, { EDataFormat::Pretty, @@ -507,13 +512,49 @@ void TCommandExecuteQuery::PrintDataQueryResponse(NTable::TDataQueryResult& resu } } // TResultSetPrinter destructor should be called before printing stats + TMaybe statsStr; + TMaybe plan; + TMaybe ast; + TMaybe meta; + const std::optional& stats = result.GetStats(); if (stats.has_value()) { - Cout << Endl << "Statistics:" << Endl << stats->ToString(); - PrintFlameGraph(stats->GetPlan()); + meta = stats->GetMeta(); + plan = stats->GetPlan(); + ast = stats->GetAst(); + statsStr = stats->ToString(); + Cout << Endl << "Statistics:" << Endl << statsStr; + if (meta) { + Cout << Endl << "Meta:" << Endl << NJson::PrettifyJson(*meta, true) << Endl; + } + PrintFlameGraph(plan); } - if (FlameGraphPath && !stats.has_value()) - { + + if (!DiagnosticsFile.empty()) { + TFileOutput file(DiagnosticsFile + ".json"); + + NJson::TJsonValue diagnosticsJson(NJson::JSON_MAP); + + if (statsStr) { + diagnosticsJson.InsertValue("stats", *statsStr); + } + if (ast) { + diagnosticsJson.InsertValue("ast", *ast); + } + if (plan) { + NJson::TJsonValue planJson; + NJson::ReadJsonTree(*plan, &planJson, true); + diagnosticsJson.InsertValue("plan", planJson); + } + if (meta) { + NJson::TJsonValue metaJson; + NJson::ReadJsonTree(*meta, &metaJson, true); + diagnosticsJson.InsertValue("meta", metaJson); + } + file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), false); + } + + if (FlameGraphPath && !stats.has_value()) { Cout << Endl << "Flame graph is available for full or profile stats only" << Endl; } } @@ -732,7 +773,8 @@ template bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { TMaybe stats; std::optional fullStats; - TString diagnostics; + TMaybe meta; + TMaybe ast; { TResultSetPrinter printer(OutputFormat, &IsInterrupted); @@ -749,10 +791,12 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { if (HasStats(streamPart)) { const auto& queryStats = GetStats(streamPart); stats = queryStats.ToString(); + ast = queryStats.GetAst(); if (queryStats.GetPlan()) { fullStats = queryStats.GetPlan(); } + meta = queryStats.GetMeta(); } } } // TResultSetPrinter destructor should be called before printing stats @@ -768,6 +812,35 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { queryPlanPrinter.Print(TString{*fullStats}); } + if (meta) { + Cout << Endl << "Meta:" << Endl << NJson::PrettifyJson(*meta, true) << Endl;; + } + + if (!DiagnosticsFile.empty()) { + TFileOutput file(TStringBuilder() << DiagnosticsFile << ".json"); + + NJson::TJsonValue diagnosticsJson(NJson::JSON_MAP); + + if (stats) { + diagnosticsJson.InsertValue("stats", *stats); + } + if (ast) { + diagnosticsJson.InsertValue("ast", *ast); + } + if (fullStats) { + NJson::TJsonValue planJson; + NJson::ReadJsonTree(*fullStats, &planJson, true); + diagnosticsJson.InsertValue("plan", planJson); + } + if (meta) { + NJson::TJsonValue metaJson; + NJson::ReadJsonTree(*meta, &metaJson, true); + metaJson.InsertValue("query_text", EscapeC(Query)); + diagnosticsJson.InsertValue("meta", metaJson); + } + file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), false); + } + PrintFlameGraph(fullStats); if (IsInterrupted()) { diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.h b/ydb/public/lib/ydb_cli/commands/ydb_service_table.h index 041543abb3ef..a1c690708efe 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.h @@ -123,6 +123,7 @@ class TCommandExecuteQuery : public TTableCommand, TCommandQueryBase, TCommandWi TString TxMode; TString QueryType; bool BasicStats = false; + TString DiagnosticsFile; }; class TCommandExplain : public TTableCommand, public TCommandWithOutput, TCommandQueryBase, TInterruptibleCommand { diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index d98c9d6a999a..346b4e2655a6 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include namespace NYdb { @@ -205,11 +206,12 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { const auto& queryStats = *streamPart.GetStats(); stats = queryStats.ToString(); ast = queryStats.GetAst(); + meta = queryStats.GetMeta(); if (queryStats.GetPlan()) { plan = queryStats.GetPlan(); } - diagnostics = queryStats.GetDiagnostics(); + meta = queryStats.GetMeta(); } } } // TResultSetPrinter destructor should be called before printing stats @@ -228,8 +230,8 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { Cout << Endl << "Statistics:" << Endl << *stats; } - if (diagnostics) { - Cout << Endl << "Diagnostics:" << Endl << NJson::PrettifyJson(*diagnostics, true) << Endl;; + if (meta) { + Cout << Endl << "Meta:" << Endl << NJson::PrettifyJson(*meta, true) << Endl;; } if (plan) { @@ -246,7 +248,7 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { } if (!DiagnosticsFile.empty()) { - TFileOutput file(DiagnosticsFile); + TFileOutput file(TStringBuilder() << DiagnosticsFile << ".json"); NJson::TJsonValue diagnosticsJson(NJson::JSON_MAP); @@ -261,9 +263,10 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { NJson::ReadJsonTree(*plan, &planJson, true); diagnosticsJson.InsertValue("plan", planJson); } - if (diagnostics) { + if (meta) { NJson::TJsonValue metaJson; - NJson::ReadJsonTree(*diagnostics, &metaJson, true); + NJson::ReadJsonTree(*meta, &metaJson, true); + metaJson.InsertValue("query_text", EscapeC(Query)); diagnosticsJson.InsertValue("meta", metaJson); } file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), false); diff --git a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp index 20010fb05eca..971712a474d5 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/impl/exec_query.cpp @@ -148,7 +148,6 @@ struct TExecuteQueryBuffer : public TThrRefBase, TNonCopyable { TVector ResultSets_; TMaybe Stats_; TMaybe Tx_; - TString Diagnostics_; void Next() { TPtr self(this); diff --git a/ydb/public/sdk/cpp/client/ydb_query/stats.cpp b/ydb/public/sdk/cpp/client/ydb_query/stats.cpp index f4577d6b6761..253eef7d5e9e 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/stats.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/stats.cpp @@ -29,7 +29,7 @@ TString TExecStats::ToString(bool withPlan) const { if (!withPlan) { proto.clear_query_plan(); proto.clear_query_ast(); - proto.clear_query_diagnostics(); + proto.clear_query_meta(); } TString res; @@ -57,14 +57,14 @@ TMaybe TExecStats::GetAst() const { return proto.query_ast(); } -TMaybe TExecStats::GetDiagnostics() const { +TMaybe TExecStats::GetMeta() const { auto proto = Impl_->Proto; - if (proto.query_diagnostics().empty()) { + if (proto.query_meta().empty()) { return {}; } - return proto.query_diagnostics(); + return proto.query_meta(); } TDuration TExecStats::GetTotalDuration() const { diff --git a/ydb/public/sdk/cpp/client/ydb_query/stats.h b/ydb/public/sdk/cpp/client/ydb_query/stats.h index e4d2ea2a0a50..ad11c7bd1e32 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/stats.h +++ b/ydb/public/sdk/cpp/client/ydb_query/stats.h @@ -33,7 +33,7 @@ class TExecStats { TMaybe GetPlan() const; TMaybe GetAst() const; - TMaybe GetDiagnostics() const; + TMaybe GetMeta() const; TDuration GetTotalDuration() const; TDuration GetTotalCpuTime() const; diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.cpp b/ydb/public/sdk/cpp/client/ydb_table/table.cpp index be9e3925fe3c..aa9230b27ed3 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/table.cpp @@ -2211,9 +2211,9 @@ const TString TDataQueryResult::GetQueryPlan() const { } } -const TString TDataQueryResult::GetDiagnostics() const { +const TString TDataQueryResult::GetMeta() const { if (QueryStats_.Defined()) { - return NYdb::TProtoAccessor::GetProto(*QueryStats_.Get()).query_diagnostics(); + return NYdb::TProtoAccessor::GetProto(*QueryStats_.Get()).query_meta(); } else { return ""; } diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.h b/ydb/public/sdk/cpp/client/ydb_table/table.h index a963ec614ead..e5baa9711eb5 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.h +++ b/ydb/public/sdk/cpp/client/ydb_table/table.h @@ -2049,7 +2049,7 @@ class TDataQueryResult : public TStatus { const TString GetQueryPlan() const; - const TString GetDiagnostics() const; + const TString GetMeta() const; private: TMaybe Transaction_; From 5e28ea458282d428f0e81b22ab5e51798c290304 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 14 Feb 2025 01:12:34 +0300 Subject: [PATCH 12/22] Fixes --- ydb/public/sdk/cpp/client/ydb_table/table.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.h b/ydb/public/sdk/cpp/client/ydb_table/table.h index e5baa9711eb5..141872621749 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.h +++ b/ydb/public/sdk/cpp/client/ydb_table/table.h @@ -1168,14 +1168,10 @@ struct TStreamExecScanQuerySettings : public TRequestSettings>>>>>> Fixes }; class TSession; From 55b4a87d19f2c9205329f7be279134c2860890ca Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 14 Feb 2025 09:40:17 +0300 Subject: [PATCH 13/22] Fixes --- ydb/core/kqp/common/compilation/events.h | 16 ++-- ydb/core/kqp/common/compilation/result.h | 11 ++- ydb/core/kqp/common/events/query.h | 2 +- .../kqp/compile_service/kqp_compile_actor.cpp | 16 ++-- .../compile_service/kqp_compile_service.cpp | 27 +++--- .../kqp/compile_service/kqp_compile_service.h | 1 - .../kqp/session_actor/kqp_query_state.cpp | 16 ++-- ydb/core/kqp/ut/indexes/kqp_indexes_ut.cpp | 4 +- ydb/core/kqp/ut/olap/kqp_olap_ut.cpp | 5 +- ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp | 87 ++++++++++++++++++- ydb/public/sdk/cpp/client/ydb_table/table.h | 2 - .../include/ydb-cpp-sdk/client/query/stats.h | 1 + .../client/table/query_stats/stats.h | 2 +- .../include/ydb-cpp-sdk/client/table/table.h | 18 ++-- ydb/public/sdk/cpp/src/client/query/stats.cpp | 11 +++ .../sdk/cpp/src/client/table/impl/readers.cpp | 7 +- .../src/client/table/impl/table_client.cpp | 1 - ydb/public/sdk/cpp/src/client/table/table.cpp | 8 ++ 18 files changed, 165 insertions(+), 70 deletions(-) diff --git a/ydb/core/kqp/common/compilation/events.h b/ydb/core/kqp/common/compilation/events.h index 1799c6800647..2daa210243db 100644 --- a/ydb/core/kqp/common/compilation/events.h +++ b/ydb/core/kqp/common/compilation/events.h @@ -16,7 +16,7 @@ namespace NKikimr::NKqp::NPrivateEvents { struct TEvCompileRequest: public TEventLocal { TEvCompileRequest(const TIntrusiveConstPtr& userToken, const TString& clientAddress, const TMaybe& uid, - TMaybe&& query, bool keepInCache, NKikimrKqp::EQueryAction queryAction, bool perStatementResult, TInstant deadline, + TMaybe&& query, bool keepInCache, bool isQueryActionPrepare, bool perStatementResult, TInstant deadline, TKqpDbCountersPtr dbCounters, const TGUCSettings::TPtr& gUCSettings, const TMaybe& applicationName, std::shared_ptr> intrestedInResult, const TIntrusivePtr& userRequestContext, NLWTrace::TOrbit orbit = {}, TKqpTempTablesState::TConstPtr tempTablesState = nullptr, bool collectDiagnostics = false, TMaybe queryAst = Nothing(), @@ -26,7 +26,7 @@ struct TEvCompileRequest: public TEventLocal Uid; TMaybe Query; bool KeepInCache = false; - NKikimrKqp::EQueryAction QueryAction; + bool IsQueryActionPrepare = false; bool PerStatementResult = false; // it is allowed for local event to use absolute time (TInstant) instead of time interval (TDuration) TInstant Deadline; @@ -76,7 +76,7 @@ struct TEvCompileRequest: public TEventLocal { TEvRecompileRequest(const TIntrusiveConstPtr& userToken, const TString& clientAddress, const TString& uid, - const TMaybe& query, NKikimrKqp::EQueryAction queryAction, TInstant deadline, + const TMaybe& query, bool isQueryActionPrepare, TInstant deadline, TKqpDbCountersPtr dbCounters, const TGUCSettings::TPtr& gUCSettings, const TMaybe& applicationName, std::shared_ptr> intrestedInResult, const TIntrusivePtr& userRequestContext, NLWTrace::TOrbit orbit = {}, TKqpTempTablesState::TConstPtr tempTablesState = nullptr, TMaybe queryAst = Nothing(), @@ -85,7 +85,7 @@ struct TEvRecompileRequest: public TEventLocal Query; - NKikimrKqp::EQueryAction QueryAction; + bool IsQueryActionPrepare = false; TInstant Deadline; TKqpDbCountersPtr DbCounters; @@ -126,16 +126,14 @@ struct TEvRecompileRequest: public TEventLocal { - TEvCompileResponse(const TKqpCompileResult::TConstPtr& compileResult, NLWTrace::TOrbit orbit = {}, const std::optional& replayMessage = std::nullopt) + TEvCompileResponse(const TKqpCompileResult::TConstPtr& compileResult, NLWTrace::TOrbit orbit = {}) : CompileResult(compileResult) - , ReplayMessage(replayMessage) , Orbit(std::move(orbit)) { } TKqpCompileResult::TConstPtr CompileResult; TKqpStatsCompile Stats; std::optional ReplayMessage; - std::optional ReplayMessageUserView; NLWTrace::TOrbit Orbit; }; diff --git a/ydb/core/kqp/common/compilation/result.h b/ydb/core/kqp/common/compilation/result.h index f7206fceb19e..a87294c8e854 100644 --- a/ydb/core/kqp/common/compilation/result.h +++ b/ydb/core/kqp/common/compilation/result.h @@ -16,7 +16,7 @@ struct TKqpCompileResult { TKqpCompileResult(const TString& uid, const Ydb::StatusIds::StatusCode& status, const NYql::TIssues& issues, ETableReadType maxReadType, TMaybe query = {}, TMaybe queryAst = {}, - bool needToSplit = false, const TMaybe& commandTagName = {}) + bool needToSplit = false, const TMaybe& commandTagName = {}, const TMaybe& replayMessageUserView = {}) : Status(status) , Issues(issues) , Query(std::move(query)) @@ -24,13 +24,14 @@ struct TKqpCompileResult { , MaxReadType(maxReadType) , QueryAst(std::move(queryAst)) , NeedToSplit(needToSplit) - , CommandTagName(commandTagName) {} + , CommandTagName(commandTagName) + , ReplayMessageUserView(replayMessageUserView) {} static std::shared_ptr Make(const TString& uid, const Ydb::StatusIds::StatusCode& status, const NYql::TIssues& issues, ETableReadType maxReadType, TMaybe query = {}, - TMaybe queryAst = {}, bool needToSplit = false, const TMaybe& commandTagName = {}) + TMaybe queryAst = {}, bool needToSplit = false, const TMaybe& commandTagName = {}, const TMaybe& replayMessageUserView = {}) { - return std::make_shared(uid, status, issues, maxReadType, std::move(query), std::move(queryAst), needToSplit, commandTagName); + return std::make_shared(uid, status, issues, maxReadType, std::move(query), std::move(queryAst), needToSplit, commandTagName, replayMessageUserView); } std::shared_ptr GetAst() const; @@ -47,6 +48,8 @@ struct TKqpCompileResult { bool NeedToSplit = false; TMaybe CommandTagName = {}; + TMaybe ReplayMessageUserView; + std::shared_ptr PreparedQuery; }; diff --git a/ydb/core/kqp/common/events/query.h b/ydb/core/kqp/common/events/query.h index 618aa74de9f0..19aad90211f2 100644 --- a/ydb/core/kqp/common/events/query.h +++ b/ydb/core/kqp/common/events/query.h @@ -282,7 +282,7 @@ struct TEvQueryRequest: public NActors::TEventLocalGetCollectDiagnostics(); + return Record.GetRequest().GetCollectDiagnostics(); } ui32 CalculateSerializedSize() const override { diff --git a/ydb/core/kqp/compile_service/kqp_compile_actor.cpp b/ydb/core/kqp/compile_service/kqp_compile_actor.cpp index d02b1af25003..91cc89b8f5bf 100644 --- a/ydb/core/kqp/compile_service/kqp_compile_actor.cpp +++ b/ydb/core/kqp/compile_service/kqp_compile_actor.cpp @@ -52,7 +52,7 @@ class TKqpCompileActor : public TActorBootstrapped { TKqpDbCountersPtr dbCounters, std::optional federatedQuerySetup, const TIntrusivePtr& userRequestContext, NWilson::TTraceId traceId, TKqpTempTablesState::TConstPtr tempTablesState, bool collectFullDiagnostics, - bool perStatementResult, NKikimrKqp::EQueryAction queryAction, + bool perStatementResult, ECompileActorAction compileAction, TMaybe queryAst, NYql::TExprContext* splitCtx, NYql::TExprNode::TPtr splitExpr) @@ -75,7 +75,6 @@ class TKqpCompileActor : public TActorBootstrapped { , CompileActorSpan(TWilsonKqp::CompileActor, std::move(traceId), "CompileActor") , TempTablesState(std::move(tempTablesState)) , CollectFullDiagnostics(collectFullDiagnostics) - , QueryAction(queryAction) , CompileAction(compileAction) , QueryAst(std::move(queryAst)) , SplitCtx(splitCtx) @@ -365,9 +364,6 @@ class TKqpCompileActor : public TActorBootstrapped { replayMessage.InsertValue("query_syntax", ToString(Config->_KqpYqlSyntaxVersion.Get().GetRef())); replayMessage.InsertValue("query_database", QueryId.Database); replayMessage.InsertValue("query_cluster", QueryId.Cluster); - if (QueryAction == NKikimrKqp::QUERY_ACTION_EXPLAIN) { - replayMessage.InsertValue("query_plan", queryPlan); - } replayMessage.InsertValue("query_type", ToString(QueryId.Settings.QueryType)); if (CollectFullDiagnostics) { @@ -382,6 +378,7 @@ class TKqpCompileActor : public TActorBootstrapped { ReplayMessageUserView = NJson::WriteJson(replayMessage, /*formatOutput*/ false); } + replayMessage.InsertValue("query_plan", queryPlan); replayMessage.InsertValue("query_text", EscapeC(QueryId.Text)); replayMessage.InsertValue("table_metadata", TString(NJson::WriteJson(tablesMeta, false))); replayMessage.InsertValue("table_meta_serialization_type", EMetaSerializationType::EncodedProto); @@ -404,10 +401,12 @@ class TKqpCompileActor : public TActorBootstrapped { << ", issues: " << KqpCompileResult->Issues.ToString() << ", uid: " << KqpCompileResult->Uid); + if (ReplayMessageUserView) { + KqpCompileResult->ReplayMessageUserView = std::move(*ReplayMessageUserView); + } auto responseEv = MakeHolder(KqpCompileResult); responseEv->ReplayMessage = std::move(ReplayMessage); - responseEv->ReplayMessageUserView = std::move(ReplayMessageUserView); ReplayMessage = std::nullopt; ReplayMessageUserView = std::nullopt; auto& stats = responseEv->Stats; @@ -616,7 +615,6 @@ class TKqpCompileActor : public TActorBootstrapped { bool CollectFullDiagnostics; bool PerStatementResult; - NKikimrKqp::EQueryAction QueryAction; ECompileActorAction CompileAction; TMaybe QueryAst; @@ -666,7 +664,7 @@ IActor* CreateKqpCompileActor(const TActorId& owner, const TKqpSettings::TConstP const TString& uid, const TKqpQueryId& query, const TIntrusiveConstPtr& userToken, const TString& clientAddress, std::optional federatedQuerySetup, TKqpDbCountersPtr dbCounters, const TGUCSettings::TPtr& gUCSettings, const TMaybe& applicationName, const TIntrusivePtr& userRequestContext, - NWilson::TTraceId traceId, TKqpTempTablesState::TConstPtr tempTablesState, NKikimrKqp::EQueryAction queryAction, + NWilson::TTraceId traceId, TKqpTempTablesState::TConstPtr tempTablesState, ECompileActorAction compileAction, TMaybe queryAst, bool collectFullDiagnostics, bool perStatementResult, NYql::TExprContext* splitCtx, NYql::TExprNode::TPtr splitExpr) { @@ -675,7 +673,7 @@ IActor* CreateKqpCompileActor(const TActorId& owner, const TKqpSettings::TConstP uid, query, userToken, clientAddress, dbCounters, federatedQuerySetup, userRequestContext, std::move(traceId), std::move(tempTablesState), collectFullDiagnostics, - perStatementResult, queryAction, compileAction, std::move(queryAst), + perStatementResult, compileAction, std::move(queryAst), splitCtx, splitExpr); } diff --git a/ydb/core/kqp/compile_service/kqp_compile_service.cpp b/ydb/core/kqp/compile_service/kqp_compile_service.cpp index fcb247e3e838..b16ccd9b7c11 100644 --- a/ydb/core/kqp/compile_service/kqp_compile_service.cpp +++ b/ydb/core/kqp/compile_service/kqp_compile_service.cpp @@ -29,17 +29,17 @@ using namespace NYql; struct TKqpCompileSettings { - TKqpCompileSettings(bool keepInCache, NKikimrKqp::EQueryAction queryAction, bool perStatementResult, + TKqpCompileSettings(bool keepInCache, bool isQueryActionPrepare, bool perStatementResult, const TInstant& deadline, ECompileActorAction action = ECompileActorAction::COMPILE) : KeepInCache(keepInCache) - , QueryAction(queryAction) + , IsQueryActionPrepare(isQueryActionPrepare) , PerStatementResult(perStatementResult) , Deadline(deadline) , Action(action) {} bool KeepInCache; - NKikimrKqp::EQueryAction QueryAction; + bool IsQueryActionPrepare; bool PerStatementResult; TInstant Deadline; ECompileActorAction Action; @@ -461,7 +461,7 @@ class TKqpCompileService : public TActorBootstrapped { TKqpCompileSettings compileSettings( request.KeepInCache, - request.QueryAction, + request.IsQueryActionPrepare, request.PerStatementResult, request.Deadline, ev->Get()->Split @@ -529,7 +529,7 @@ class TKqpCompileService : public TActorBootstrapped { TKqpCompileSettings compileSettings( true, - request.QueryAction, + request.IsQueryActionPrepare, false, request.Deadline, ev->Get()->Split @@ -610,7 +610,7 @@ class TKqpCompileService : public TActorBootstrapped { if (compileResult->NeedToSplit) { Reply(compileRequest.Sender, compileResult, compileStats, ctx, - compileRequest.Cookie, std::move(compileRequest.Orbit), std::move(compileRequest.CompileServiceSpan), (CollectDiagnostics ? ev->Get()->ReplayMessageUserView : std::nullopt)); + compileRequest.Cookie, std::move(compileRequest.Orbit), std::move(compileRequest.CompileServiceSpan)); ProcessQueue(ctx); return; } @@ -623,7 +623,7 @@ class TKqpCompileService : public TActorBootstrapped { try { if (compileResult->Status == Ydb::StatusIds::SUCCESS) { if (!hasTempTablesNameClashes) { - UpdateQueryCache(ctx, compileResult, keepInCache, compileRequest.CompileSettings.QueryAction == NKikimrKqp::QUERY_ACTION_PREPARE, isPerStatementExecution); + UpdateQueryCache(ctx, compileResult, keepInCache, compileRequest.CompileSettings.IsQueryActionPrepare, isPerStatementExecution); } if (ev->Get()->ReplayMessage && !QueryReplayBackend->IsNull()) { @@ -635,7 +635,7 @@ class TKqpCompileService : public TActorBootstrapped { for (auto& request : requests) { LWTRACK(KqpCompileServiceGetCompilation, request.Orbit, request.Query.UserSid, compileActorId.ToString()); Reply(request.Sender, compileResult, compileStats, ctx, - request.Cookie, std::move(request.Orbit), std::move(request.CompileServiceSpan), (CollectDiagnostics ? ev->Get()->ReplayMessageUserView : std::nullopt)); + request.Cookie, std::move(request.Orbit), std::move(request.CompileServiceSpan)); } } else { if (!hasTempTablesNameClashes) { @@ -647,7 +647,7 @@ class TKqpCompileService : public TActorBootstrapped { LWTRACK(KqpCompileServiceGetCompilation, compileRequest.Orbit, compileRequest.Query.UserSid, compileActorId.ToString()); Reply(compileRequest.Sender, compileResult, compileStats, ctx, - compileRequest.Cookie, std::move(compileRequest.Orbit), std::move(compileRequest.CompileServiceSpan), (CollectDiagnostics ? ev->Get()->ReplayMessageUserView : std::nullopt)); + compileRequest.Cookie, std::move(compileRequest.Orbit), std::move(compileRequest.CompileServiceSpan)); } catch (const std::exception& e) { LogException("TEvCompileResponse", ev->Sender, e, ctx); @@ -809,7 +809,8 @@ class TKqpCompileService : public TActorBootstrapped { if (compileResult->GetAst() && QueryCache->FindByAst(query, *compileResult->GetAst(), keepInCache)) { return false; } - auto newCompileResult = TKqpCompileResult::Make(CreateGuidAsString(), compileResult->Status, compileResult->Issues, compileResult->MaxReadType, std::move(query), compileResult->QueryAst); + auto newCompileResult = TKqpCompileResult::Make(CreateGuidAsString(), compileResult->Status, compileResult->Issues, compileResult->MaxReadType, std::move(query), compileResult->QueryAst, + false, {}, compileResult->ReplayMessageUserView); newCompileResult->AllowCache = compileResult->AllowCache; newCompileResult->PreparedQuery = compileResult->PreparedQuery; LOG_DEBUG_S(ctx, NKikimrServices::KQP_COMPILE_SERVICE, "Insert preparing query with params, queryId: " << query.SerializeToString()); @@ -846,7 +847,7 @@ class TKqpCompileService : public TActorBootstrapped { void StartCompilation(TKqpCompileRequest&& request, const TActorContext& ctx) { auto compileActor = CreateKqpCompileActor(ctx.SelfID, KqpSettings, TableServiceConfig, QueryServiceConfig, ModuleResolverState, Counters, request.Uid, request.Query, request.UserToken, request.ClientAddress, FederatedQuerySetup, request.DbCounters, request.GUCSettings, request.ApplicationName, request.UserRequestContext, - request.CompileServiceSpan.GetTraceId(), request.TempTablesState, request.CompileSettings.QueryAction, request.CompileSettings.Action, std::move(request.QueryAst), CollectDiagnostics, + request.CompileServiceSpan.GetTraceId(), request.TempTablesState, request.CompileSettings.Action, std::move(request.QueryAst), CollectDiagnostics, request.CompileSettings.PerStatementResult, request.SplitCtx, request.SplitExpr); auto compileActorId = ctx.Register(compileActor, TMailboxType::HTSwap, AppData(ctx)->UserPoolId); @@ -865,7 +866,7 @@ class TKqpCompileService : public TActorBootstrapped { void Reply(const TActorId& sender, const TKqpCompileResult::TConstPtr& compileResult, const TKqpStatsCompile& compileStats, const TActorContext& ctx, ui64 cookie, - NLWTrace::TOrbit orbit, NWilson::TSpan span, const std::optional& replayMessage = std::nullopt) + NLWTrace::TOrbit orbit, NWilson::TSpan span) { const auto& query = compileResult->Query; LWTRACK(KqpCompileServiceReply, @@ -878,7 +879,7 @@ class TKqpCompileService : public TActorBootstrapped { << ", queryUid: " << compileResult->Uid << ", status:" << compileResult->Status); - auto responseEv = MakeHolder(compileResult, std::move(orbit), replayMessage); + auto responseEv = MakeHolder(compileResult, std::move(orbit)); responseEv->Stats = compileStats; if (span) { diff --git a/ydb/core/kqp/compile_service/kqp_compile_service.h b/ydb/core/kqp/compile_service/kqp_compile_service.h index 9031019ff4a6..853ba5a35910 100644 --- a/ydb/core/kqp/compile_service/kqp_compile_service.h +++ b/ydb/core/kqp/compile_service/kqp_compile_service.h @@ -166,7 +166,6 @@ IActor* CreateKqpCompileActor(const TActorId& owner, const TKqpSettings::TConstP TKqpDbCountersPtr dbCounters, const TGUCSettings::TPtr& gUCSettings, const TMaybe& applicationName, const TIntrusivePtr& userRequestContext, NWilson::TTraceId traceId = {}, TKqpTempTablesState::TConstPtr tempTablesState = nullptr, - NKikimrKqp::EQueryAction queryAction = NKikimrKqp::QUERY_ACTION_EXECUTE, ECompileActorAction compileAction = ECompileActorAction::COMPILE, TMaybe queryAst = {}, bool collectFullDiagnostics = false, diff --git a/ydb/core/kqp/session_actor/kqp_query_state.cpp b/ydb/core/kqp/session_actor/kqp_query_state.cpp index 1f05419948bd..eaf20f4f23d1 100644 --- a/ydb/core/kqp/session_actor/kqp_query_state.cpp +++ b/ydb/core/kqp/session_actor/kqp_query_state.cpp @@ -143,9 +143,6 @@ bool TKqpQueryState::SaveAndCheckCompileResult(TEvKqp::TEvCompileResponse* ev) { return false; } Orbit = std::move(ev->Orbit); - if (ev->ReplayMessage) { - ReplayMessage = *ev->ReplayMessage; - } return true; } @@ -160,6 +157,10 @@ bool TKqpQueryState::SaveAndCheckCompileResult(TKqpCompileResult::TConstPtr comp return false; } + if (compileResult->ReplayMessageUserView && GetCollectDiagnostics()) { + ReplayMessage = *compileResult->ReplayMessageUserView; + } + YQL_ENSURE(CompileResult->PreparedQuery); const ui32 compiledVersion = CompileResult->PreparedQuery->GetVersion(); YQL_ENSURE(compiledVersion == NKikimrKqp::TPreparedQuery::VERSION_PHYSICAL_V1, @@ -300,6 +301,8 @@ std::unique_ptr TKqpQueryState::BuildCompileRequest(s compileDeadline = Min(compileDeadline, QueryDeadlines.CancelAt); } + bool isQueryActionPrepare = GetAction() == NKikimrKqp::QUERY_ACTION_PREPARE; + TMaybe statementAst; if (!Statements.empty()) { YQL_ENSURE(CurrentStatementId < Statements.size()); @@ -307,7 +310,7 @@ std::unique_ptr TKqpQueryState::BuildCompileRequest(s } return std::make_unique(UserToken, ClientAddress, uid, std::move(query), keepInCache, - GetAction(), perStatementResult, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), + isQueryActionPrepare, perStatementResult, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), UserRequestContext, std::move(Orbit), TempTablesState, GetCollectDiagnostics(), statementAst); } @@ -342,11 +345,12 @@ std::unique_ptr TKqpQueryState::BuildReCompileReque } auto compileDeadline = QueryDeadlines.TimeoutAt; + bool isQueryActionPrepare = GetAction() == NKikimrKqp::QUERY_ACTION_PREPARE; if (QueryDeadlines.CancelAt) { compileDeadline = Min(compileDeadline, QueryDeadlines.CancelAt); } - return std::make_unique(UserToken, ClientAddress, CompileResult->Uid, query, GetAction(), + return std::make_unique(UserToken, ClientAddress, CompileResult->Uid, query, isQueryActionPrepare, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), UserRequestContext, std::move(Orbit), TempTablesState, CompileResult->QueryAst); } @@ -389,7 +393,7 @@ std::unique_ptr TKqpQueryState::BuildCompileSplittedR } return std::make_unique(UserToken, ClientAddress, uid, std::move(query), false, - NKikimrKqp::QUERY_ACTION_EXECUTE, perStatementResult, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), + false, perStatementResult, compileDeadline, DbCounters, gUCSettingsPtr, ApplicationName, std::move(cookie), UserRequestContext, std::move(Orbit), TempTablesState, GetCollectDiagnostics(), statementAst, false, SplittedCtx.get(), SplittedExprs.at(NextSplittedExpr)); } diff --git a/ydb/core/kqp/ut/indexes/kqp_indexes_ut.cpp b/ydb/core/kqp/ut/indexes/kqp_indexes_ut.cpp index 8a56cf6c3cf5..f12cff231775 100644 --- a/ydb/core/kqp/ut/indexes/kqp_indexes_ut.cpp +++ b/ydb/core/kqp/ut/indexes/kqp_indexes_ut.cpp @@ -2698,7 +2698,7 @@ Y_UNIT_TEST_SUITE(KqpIndexes) { UNIT_ASSERT_C(value.IsMap(), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("version"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_text"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(!value.Has("query_text"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Diagnostics: table_metadata type should be an array"); @@ -2706,7 +2706,7 @@ Y_UNIT_TEST_SUITE(KqpIndexes) { UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Diagnostics"); - UNIT_ASSERT_C(value.Has("query_plan"), "Incorrect Diagnostics"); + UNIT_ASSERT_C(!value.Has("query_plan"), "Incorrect Diagnostics"); UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Diagnostics"); } diff --git a/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp b/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp index c31081525192..a592340f6e73 100644 --- a/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp +++ b/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp @@ -221,7 +221,7 @@ Y_UNIT_TEST_SUITE(KqpOlap) { Y_UNIT_TEST(AlterObjectDisabled) { auto settings = TKikimrSettings() .SetWithSampleTables(false); - TKikimrRunner kikimr(settings); + TKikimrRunner kikimr(settings); TLocalHelper(kikimr).CreateTestOlapTableWithoutStore(); { @@ -426,6 +426,7 @@ Y_UNIT_TEST_SUITE(KqpOlap) { UNIT_ASSERT_C(jsonMeta.IsMap(), "Incorrect Meta"); UNIT_ASSERT_C(jsonMeta.Has("query_id"), "Incorrect Meta"); UNIT_ASSERT_C(jsonMeta.Has("version"), "Incorrect Meta"); + UNIT_ASSERT_C(!jsonMeta.Has("query_text"), "Incorrect Meta"); UNIT_ASSERT_C(jsonMeta.Has("query_parameter_types"), "Incorrect Meta"); UNIT_ASSERT_C(jsonMeta.Has("table_metadata"), "Incorrect Meta"); UNIT_ASSERT_C(jsonMeta.Has("created_at"), "Incorrect Meta"); @@ -2820,7 +2821,7 @@ Y_UNIT_TEST_SUITE(KqpOlap) { { auto alterQuery = R"(ALTER OBJECT `/Root/olapStore` (TYPE TABLESTORE) SET (ACTION=UPSERT_OPTIONS, `COMPACTION_PLANNER.CLASS_NAME`=`lc-buckets`, `COMPACTION_PLANNER.FEATURES`=` - {"levels" : [{"class_name" : "Zero", "expected_blobs_size" : 1, "portions_count_available" : 3}, + {"levels" : [{"class_name" : "Zero", "expected_blobs_size" : 1, "portions_count_available" : 3}, {"class_name" : "Zero"}]}`); )"; auto result = session.ExecuteQuery(alterQuery, NQuery::TTxControl::NoTx()).GetValueSync(); diff --git a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp index b6a461469135..7daf90f727f3 100644 --- a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp +++ b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp @@ -673,6 +673,7 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { UNIT_ASSERT_C(value.IsMap(), "Incorrect Meta"); UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Meta"); + UNIT_ASSERT_C(!value.Has("query_text"), "Incorrect Meta"); UNIT_ASSERT_C(value.Has("version"), "Incorrect Meta"); UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Meta"); UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Meta"); @@ -698,6 +699,42 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { auto& stats = NYdb::TProtoAccessor::GetProto(*result.GetStats()); UNIT_ASSERT_C(stats.query_meta().empty(), "Query result Meta should be empty, but it's not"); } + + { + TExecuteQuerySettings settings; + settings.ExecMode(EExecMode::Explain); + + auto result = db.ExecuteQuery(R"( + SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; + )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetResultSets().empty()); + + UNIT_ASSERT(result.GetStats().has_value()); + + auto& stats = NYdb::TProtoAccessor::GetProto(*result.GetStats()); + UNIT_ASSERT_C(!stats.query_ast().empty(), "Query result ast is empty"); + UNIT_ASSERT_C(!stats.query_meta().empty(), "Query result meta is empty"); + + TStringStream in; + in << stats.query_meta(); + NJson::TJsonValue value; + ReadJsonTree(&in, &value); + + UNIT_ASSERT_C(value.IsMap(), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("version"), "Incorrect Meta"); + UNIT_ASSERT_C(!value.Has("query_text"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Meta"); + UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Meta: table_metadata type should be an array"); + UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Meta"); + UNIT_ASSERT_C(!value.Has("query_plan"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Meta"); + } } Y_UNIT_TEST(StreamExecuteCollectMeta) { @@ -722,7 +759,52 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { } const auto& execStats = streamPart.GetStats(); - if (execStats.Defined()) { + if (execStats) { + auto& stats = NYdb::TProtoAccessor::GetProto(*execStats); + statsString = stats.query_meta(); + } + } + + UNIT_ASSERT_C(!statsString.empty(), "Query result meta is empty"); + + TStringStream in; + in << statsString; + NJson::TJsonValue value; + ReadJsonTree(&in, &value); + + UNIT_ASSERT_C(value.IsMap(), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Meta"); + UNIT_ASSERT_C(!value.Has("query_text"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("version"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Meta"); + UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Meta: table_metadata type should be an array"); + UNIT_ASSERT_C(value.Has("created_at"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_syntax"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_database"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_cluster"), "Incorrect Meta"); + UNIT_ASSERT_C(!value.Has("query_plan"), "Incorrect Meta"); + UNIT_ASSERT_C(value.Has("query_type"), "Incorrect Meta"); + } + + { + auto settings = TExecuteQuerySettings().ExecMode(EExecMode::Explain); + + auto it = db.StreamExecuteQuery(R"( + SELECT Key, Value2 FROM TwoShard WHERE Value2 > 0; + )", TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(it.GetStatus(), EStatus::SUCCESS, it.GetIssues().ToString()); + + TString statsString; + for (;;) { + auto streamPart = it.ReadNext().GetValueSync(); + if (!streamPart.IsSuccess()) { + UNIT_ASSERT_C(streamPart.EOS(), streamPart.GetIssues().ToString()); + break; + } + + const auto& execStats = streamPart.GetStats(); + if (execStats) { auto& stats = NYdb::TProtoAccessor::GetProto(*execStats); statsString = stats.query_meta(); } @@ -738,6 +820,7 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { UNIT_ASSERT_C(value.IsMap(), "Incorrect Meta"); UNIT_ASSERT_C(value.Has("query_id"), "Incorrect Meta"); UNIT_ASSERT_C(value.Has("version"), "Incorrect Meta"); + UNIT_ASSERT_C(!value.Has("query_text"), "Incorrect Meta"); UNIT_ASSERT_C(value.Has("query_parameter_types"), "Incorrect Meta"); UNIT_ASSERT_C(value.Has("table_metadata"), "Incorrect Meta"); UNIT_ASSERT_C(value["table_metadata"].IsArray(), "Incorrect Meta: table_metadata type should be an array"); @@ -767,7 +850,7 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { } const auto& execStats = streamPart.GetStats(); - if (execStats.Defined()) { + if (execStats) { auto& stats = NYdb::TProtoAccessor::GetProto(*execStats); statsString = stats.query_meta(); } diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.h b/ydb/public/sdk/cpp/client/ydb_table/table.h index 141872621749..db723854cb47 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.h +++ b/ydb/public/sdk/cpp/client/ydb_table/table.h @@ -2045,8 +2045,6 @@ class TDataQueryResult : public TStatus { const TString GetQueryPlan() const; - const TString GetMeta() const; - private: TMaybe Transaction_; TVector ResultSets_; diff --git a/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/query/stats.h b/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/query/stats.h index 902478b44643..0b7ca6cc6de9 100644 --- a/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/query/stats.h +++ b/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/query/stats.h @@ -29,6 +29,7 @@ class TExecStats { std::optional GetPlan() const; std::optional GetAst() const; + std::optional GetMeta() const; TDuration GetTotalDuration() const; TDuration GetTotalCpuTime() const; diff --git a/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/query_stats/stats.h b/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/query_stats/stats.h index 38740d2dd13c..9eda4620ff5d 100644 --- a/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/query_stats/stats.h +++ b/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/query_stats/stats.h @@ -30,7 +30,7 @@ namespace NTable { enum class ECollectQueryStatsMode { None = 0, // Stats collection is disabled Basic = 1, // Aggregated stats of reads, updates and deletes per table - Full = 2, // Add per-stage execution profile and query plan on top of Basic mode + Full = 2, // Add per-stage execution profile, query plan and query meta on top of Basic mode Profile = 3 // Detailed execution stats including stats for individual tasks and channels }; diff --git a/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h b/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h index df5b02d91678..0f94a89eae71 100644 --- a/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h +++ b/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h @@ -1163,9 +1163,8 @@ struct TStreamExecScanQuerySettings : public TRequestSettings Transaction_; std::vector ResultSets_; @@ -2109,31 +2110,24 @@ class TScanQueryPart : public TStreamPartStatus { const TQueryStats& GetQueryStats() const { return *QueryStats_; } TQueryStats ExtractQueryStats() { return std::move(*QueryStats_); } - bool HasDiagnostics() const { return Diagnostics_.has_value(); } - const std::string& GetDiagnostics() const { return *Diagnostics_; } - std::string&& ExtractDiagnostics() { return std::move(*Diagnostics_); } - TScanQueryPart(TStatus&& status) : TStreamPartStatus(std::move(status)) {} - TScanQueryPart(TStatus&& status, const std::optional& queryStats, const std::optional& diagnostics) + TScanQueryPart(TStatus&& status, const std::optional& queryStats) : TStreamPartStatus(std::move(status)) , QueryStats_(queryStats) - , Diagnostics_(diagnostics) {} - TScanQueryPart(TStatus&& status, TResultSet&& resultSet, const std::optional& queryStats, const std::optional& diagnostics) + TScanQueryPart(TStatus&& status, TResultSet&& resultSet, const std::optional& queryStats) : TStreamPartStatus(std::move(status)) , ResultSet_(std::move(resultSet)) , QueryStats_(queryStats) - , Diagnostics_(diagnostics) {} private: std::optional ResultSet_; std::optional QueryStats_; - std::optional Diagnostics_; }; using TAsyncScanQueryPart = NThreading::TFuture; diff --git a/ydb/public/sdk/cpp/src/client/query/stats.cpp b/ydb/public/sdk/cpp/src/client/query/stats.cpp index e56d5533f627..5106687f23d1 100644 --- a/ydb/public/sdk/cpp/src/client/query/stats.cpp +++ b/ydb/public/sdk/cpp/src/client/query/stats.cpp @@ -31,6 +31,7 @@ std::string TExecStats::ToString(bool withPlan) const { if (!withPlan) { proto.clear_query_plan(); proto.clear_query_ast(); + proto.clear_query_meta(); } TStringType res; @@ -58,6 +59,16 @@ std::optional TExecStats::GetAst() const { return proto.query_ast(); } +std::optional TExecStats::GetMeta() const { + auto proto = Impl_->Proto; + + if (proto.query_meta().empty()) { + return {}; + } + + return proto.query_meta(); +} + TDuration TExecStats::GetTotalDuration() const { return TDuration::MicroSeconds(Impl_->Proto.total_duration_us()); } diff --git a/ydb/public/sdk/cpp/src/client/table/impl/readers.cpp b/ydb/public/sdk/cpp/src/client/table/impl/readers.cpp index acb5f6dd58b1..bc4ee7a5e6f7 100644 --- a/ydb/public/sdk/cpp/src/client/table/impl/readers.cpp +++ b/ydb/public/sdk/cpp/src/client/table/impl/readers.cpp @@ -82,19 +82,16 @@ TAsyncScanQueryPart TScanQueryPartIterator::TReaderImpl::ReadNext(std::shared_pt TPlainStatus plainStatus{clientStatus, std::move(issues), self->Endpoint_, {}}; TStatus status{std::move(plainStatus)}; std::optional queryStats; - std::optional diagnostics; if (self->Response_.result().has_query_stats()) { queryStats = TQueryStats(self->Response_.result().query_stats()); } - diagnostics = self->Response_.result().query_full_diagnostics(); - if (self->Response_.result().has_result_set()) { promise.SetValue({std::move(status), - TResultSet(std::move(*self->Response_.mutable_result()->mutable_result_set())), queryStats, diagnostics}); + TResultSet(std::move(*self->Response_.mutable_result()->mutable_result_set())), queryStats}); } else { - promise.SetValue({std::move(status), queryStats, diagnostics}); + promise.SetValue({std::move(status), queryStats}); } } }; diff --git a/ydb/public/sdk/cpp/src/client/table/impl/table_client.cpp b/ydb/public/sdk/cpp/src/client/table/impl/table_client.cpp index af68960e836a..8441f02581ef 100644 --- a/ydb/public/sdk/cpp/src/client/table/impl/table_client.cpp +++ b/ydb/public/sdk/cpp/src/client/table/impl/table_client.cpp @@ -1073,7 +1073,6 @@ TFuture> TT } request.set_collect_stats(GetStatsCollectionMode(settings.CollectQueryStats_)); - request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); auto promise = NewPromise>(); diff --git a/ydb/public/sdk/cpp/src/client/table/table.cpp b/ydb/public/sdk/cpp/src/client/table/table.cpp index 4ebdbdd794d9..5a41c6d6f877 100644 --- a/ydb/public/sdk/cpp/src/client/table/table.cpp +++ b/ydb/public/sdk/cpp/src/client/table/table.cpp @@ -2220,6 +2220,14 @@ const std::string TDataQueryResult::GetQueryPlan() const { } } +const std::string TDataQueryResult::GetMeta() const { + if (QueryStats_.has_value()) { + return NYdb::TProtoAccessor::GetProto(*QueryStats_).query_meta(); + } else { + return ""; + } +} + //////////////////////////////////////////////////////////////////////////////// TBeginTransactionResult::TBeginTransactionResult(TStatus&& status, TTransaction transaction) From dde41daed3d8517ea4fefa7645c9b8c75ef9cb69 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 14 Feb 2025 09:50:40 +0300 Subject: [PATCH 14/22] Fixes --- ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp | 8 +++++--- ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | 7 ++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index 9d28f4251673..3c1514cbe2bb 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -551,7 +551,7 @@ void TCommandExecuteQuery::PrintDataQueryResponse(NTable::TDataQueryResult& resu NJson::ReadJsonTree(*meta, &metaJson, true); diagnosticsJson.InsertValue("meta", metaJson); } - file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), false); + file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), true); } if (FlameGraphPath && !stats.has_value()) { @@ -796,7 +796,9 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { if (queryStats.GetPlan()) { fullStats = queryStats.GetPlan(); } - meta = queryStats.GetMeta(); + if (queryStats.GetMeta()) { + meta = queryStats.GetMeta(); + } } } } // TResultSetPrinter destructor should be called before printing stats @@ -838,7 +840,7 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { metaJson.InsertValue("query_text", EscapeC(Query)); diagnosticsJson.InsertValue("meta", metaJson); } - file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), false); + file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), true); } PrintFlameGraph(fullStats); diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index 346b4e2655a6..a7563a0f7b1a 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp @@ -206,12 +206,13 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { const auto& queryStats = *streamPart.GetStats(); stats = queryStats.ToString(); ast = queryStats.GetAst(); - meta = queryStats.GetMeta(); if (queryStats.GetPlan()) { plan = queryStats.GetPlan(); } - meta = queryStats.GetMeta(); + if (queryStats.GetMeta()) { + meta = queryStats.GetMeta(); + } } } } // TResultSetPrinter destructor should be called before printing stats @@ -269,7 +270,7 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { metaJson.InsertValue("query_text", EscapeC(Query)); diagnosticsJson.InsertValue("meta", metaJson); } - file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), false); + file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), true); } if (IsInterrupted()) { From 904e4cd8a01865b0b0799be0078cd022489a3a73 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 14 Feb 2025 10:06:38 +0300 Subject: [PATCH 15/22] Fixes --- .../ydb_cli/commands/ydb_service_table.cpp | 22 +++++++++++-------- ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | 2 +- ydb/public/sdk/cpp/client/ydb_query/stats.cpp | 11 ---------- ydb/public/sdk/cpp/client/ydb_query/stats.h | 1 - .../sdk/cpp/client/ydb_table/impl/readers.cpp | 5 +++-- .../client/ydb_table/impl/table_client.cpp | 1 + .../cpp/client/ydb_table/query_stats/stats.h | 2 +- ydb/public/sdk/cpp/client/ydb_table/table.cpp | 8 ------- ydb/public/sdk/cpp/client/ydb_table/table.h | 11 ++++++++-- 9 files changed, 28 insertions(+), 35 deletions(-) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index 3c1514cbe2bb..f8d036723dca 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -512,15 +512,19 @@ void TCommandExecuteQuery::PrintDataQueryResponse(NTable::TDataQueryResult& resu } } // TResultSetPrinter destructor should be called before printing stats - TMaybe statsStr; - TMaybe plan; - TMaybe ast; - TMaybe meta; + std::optional statsStr; + std::optional plan; + std::optional ast; + std::optional meta; const std::optional& stats = result.GetStats(); if (stats.has_value()) { - meta = stats->GetMeta(); - plan = stats->GetPlan(); + if (stats->GetMeta()) { + meta = stats->GetMeta(); + } + if (stats->GetPlan()) { + plan = stats->GetPlan(); + } ast = stats->GetAst(); statsStr = stats->ToString(); Cout << Endl << "Statistics:" << Endl << statsStr; @@ -771,10 +775,10 @@ int TCommandExecuteQuery::ExecuteQueryImpl(TConfig& config) { template bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { - TMaybe stats; + std::optional stats; std::optional fullStats; - TMaybe meta; - TMaybe ast; + std::optional meta; + std::optional ast; { TResultSetPrinter printer(OutputFormat, &IsInterrupted); diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index a7563a0f7b1a..dcd35f540aa6 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp @@ -188,7 +188,7 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { std::optional stats; std::optional plan; std::optional ast; - TMaybe meta; + std::optional meta; { TResultSetPrinter printer(OutputFormat, &IsInterrupted); diff --git a/ydb/public/sdk/cpp/client/ydb_query/stats.cpp b/ydb/public/sdk/cpp/client/ydb_query/stats.cpp index 253eef7d5e9e..6b5c594f9332 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/stats.cpp +++ b/ydb/public/sdk/cpp/client/ydb_query/stats.cpp @@ -29,7 +29,6 @@ TString TExecStats::ToString(bool withPlan) const { if (!withPlan) { proto.clear_query_plan(); proto.clear_query_ast(); - proto.clear_query_meta(); } TString res; @@ -57,16 +56,6 @@ TMaybe TExecStats::GetAst() const { return proto.query_ast(); } -TMaybe TExecStats::GetMeta() const { - auto proto = Impl_->Proto; - - if (proto.query_meta().empty()) { - return {}; - } - - return proto.query_meta(); -} - TDuration TExecStats::GetTotalDuration() const { return TDuration::MicroSeconds(Impl_->Proto.total_duration_us()); } diff --git a/ydb/public/sdk/cpp/client/ydb_query/stats.h b/ydb/public/sdk/cpp/client/ydb_query/stats.h index ad11c7bd1e32..a789da5a55c4 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/stats.h +++ b/ydb/public/sdk/cpp/client/ydb_query/stats.h @@ -33,7 +33,6 @@ class TExecStats { TMaybe GetPlan() const; TMaybe GetAst() const; - TMaybe GetMeta() const; TDuration GetTotalDuration() const; TDuration GetTotalCpuTime() const; diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp b/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp index 064992846c00..c76d57831105 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp @@ -82,6 +82,7 @@ TAsyncScanQueryPart TScanQueryPartIterator::TReaderImpl::ReadNext(std::shared_pt TPlainStatus plainStatus{clientStatus, std::move(issues), self->Endpoint_, {}}; TStatus status{std::move(plainStatus)}; TMaybe queryStats; + TMaybe diagnostics; if (self->Response_.result().has_query_stats()) { queryStats = TQueryStats(self->Response_.result().query_stats()); @@ -89,9 +90,9 @@ TAsyncScanQueryPart TScanQueryPartIterator::TReaderImpl::ReadNext(std::shared_pt if (self->Response_.result().has_result_set()) { promise.SetValue({std::move(status), - TResultSet(std::move(*self->Response_.mutable_result()->mutable_result_set())), queryStats}); + TResultSet(std::move(*self->Response_.mutable_result()->mutable_result_set())), queryStats, diagnostics}); } else { - promise.SetValue({std::move(status), queryStats}); + promise.SetValue({std::move(status), queryStats, diagnostics}); } } }; diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp index 8442584f830b..04fa0286ce92 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/impl/table_client.cpp @@ -1017,6 +1017,7 @@ TFuture> TT } request.set_collect_stats(GetStatsCollectionMode(settings.CollectQueryStats_)); + request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); auto promise = NewPromise>(); diff --git a/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h b/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h index 4b1bdc801997..8c8d40a52863 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h +++ b/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h @@ -35,7 +35,7 @@ namespace NTable { enum class ECollectQueryStatsMode { None = 0, // Stats collection is disabled Basic = 1, // Aggregated stats of reads, updates and deletes per table - Full = 2, // Add per-stage execution profile and diagnostics on top of Basic mode + Full = 2, // Add per-stage execution profile and query plan on top of Basic mode Profile = 3 // Detailed execution stats including stats for individual tasks and channels }; diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.cpp b/ydb/public/sdk/cpp/client/ydb_table/table.cpp index aa9230b27ed3..39708e380077 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/table.cpp @@ -2211,14 +2211,6 @@ const TString TDataQueryResult::GetQueryPlan() const { } } -const TString TDataQueryResult::GetMeta() const { - if (QueryStats_.Defined()) { - return NYdb::TProtoAccessor::GetProto(*QueryStats_.Get()).query_meta(); - } else { - return ""; - } -} - //////////////////////////////////////////////////////////////////////////////// TBeginTransactionResult::TBeginTransactionResult(TStatus&& status, TTransaction transaction) diff --git a/ydb/public/sdk/cpp/client/ydb_table/table.h b/ydb/public/sdk/cpp/client/ydb_table/table.h index db723854cb47..155e4a949565 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.h +++ b/ydb/public/sdk/cpp/client/ydb_table/table.h @@ -2117,24 +2117,31 @@ class TScanQueryPart : public TStreamPartStatus { const TQueryStats& GetQueryStats() const { return *QueryStats_; } TQueryStats ExtractQueryStats() { return std::move(*QueryStats_); } + bool HasDiagnostics() const { return Diagnostics_.Defined(); } + const TString& GetDiagnostics() const { return *Diagnostics_; } + TString&& ExtractDiagnostics() { return std::move(*Diagnostics_); } + TScanQueryPart(TStatus&& status) : TStreamPartStatus(std::move(status)) {} - TScanQueryPart(TStatus&& status, const TMaybe& queryStats) + TScanQueryPart(TStatus&& status, const TMaybe& queryStats, const TMaybe& diagnostics) : TStreamPartStatus(std::move(status)) , QueryStats_(queryStats) + , Diagnostics_(diagnostics) {} - TScanQueryPart(TStatus&& status, TResultSet&& resultSet, const TMaybe& queryStats) + TScanQueryPart(TStatus&& status, TResultSet&& resultSet, const TMaybe& queryStats, const TMaybe& diagnostics) : TStreamPartStatus(std::move(status)) , ResultSet_(std::move(resultSet)) , QueryStats_(queryStats) + , Diagnostics_(diagnostics) {} private: TMaybe ResultSet_; TMaybe QueryStats_; + TMaybe Diagnostics_; }; using TAsyncScanQueryPart = NThreading::TFuture; From e3fcd8b38931fe9609a7f58da3f62b49d533be81 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 14 Feb 2025 10:08:12 +0300 Subject: [PATCH 16/22] Fixes --- ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp b/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp index c76d57831105..cc54950d2519 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/impl/readers.cpp @@ -88,6 +88,8 @@ TAsyncScanQueryPart TScanQueryPartIterator::TReaderImpl::ReadNext(std::shared_pt queryStats = TQueryStats(self->Response_.result().query_stats()); } + diagnostics = self->Response_.result().query_full_diagnostics(); + if (self->Response_.result().has_result_set()) { promise.SetValue({std::move(status), TResultSet(std::move(*self->Response_.mutable_result()->mutable_result_set())), queryStats, diagnostics}); From 04722c8d0fe22f9aed9a170440d668e7c3daae81 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 14 Feb 2025 10:20:59 +0300 Subject: [PATCH 17/22] Fixes --- ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp | 4 ++-- ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index f8d036723dca..bb61c670e198 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -555,7 +555,7 @@ void TCommandExecuteQuery::PrintDataQueryResponse(NTable::TDataQueryResult& resu NJson::ReadJsonTree(*meta, &metaJson, true); diagnosticsJson.InsertValue("meta", metaJson); } - file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), true); + file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), false); } if (FlameGraphPath && !stats.has_value()) { @@ -844,7 +844,7 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { metaJson.InsertValue("query_text", EscapeC(Query)); diagnosticsJson.InsertValue("meta", metaJson); } - file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), true); + file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), false); } PrintFlameGraph(fullStats); diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index dcd35f540aa6..efe9231f5960 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp @@ -270,7 +270,7 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { metaJson.InsertValue("query_text", EscapeC(Query)); diagnosticsJson.InsertValue("meta", metaJson); } - file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), true); + file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), false); } if (IsInterrupted()) { From c0ff915e340706d698dcb3471984a872f7c109e6 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 14 Feb 2025 11:23:20 +0300 Subject: [PATCH 18/22] Fixes --- ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp | 8 +------- ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | 4 ---- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index bb61c670e198..ff164130d9ea 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -528,9 +528,6 @@ void TCommandExecuteQuery::PrintDataQueryResponse(NTable::TDataQueryResult& resu ast = stats->GetAst(); statsStr = stats->ToString(); Cout << Endl << "Statistics:" << Endl << statsStr; - if (meta) { - Cout << Endl << "Meta:" << Endl << NJson::PrettifyJson(*meta, true) << Endl; - } PrintFlameGraph(plan); } @@ -553,6 +550,7 @@ void TCommandExecuteQuery::PrintDataQueryResponse(NTable::TDataQueryResult& resu if (meta) { NJson::TJsonValue metaJson; NJson::ReadJsonTree(*meta, &metaJson, true); + metaJson.InsertValue("query_text", EscapeC(Query)); diagnosticsJson.InsertValue("meta", metaJson); } file << NJson::PrettifyJson(NJson::WriteJson(diagnosticsJson, true), false); @@ -818,10 +816,6 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { queryPlanPrinter.Print(TString{*fullStats}); } - if (meta) { - Cout << Endl << "Meta:" << Endl << NJson::PrettifyJson(*meta, true) << Endl;; - } - if (!DiagnosticsFile.empty()) { TFileOutput file(TStringBuilder() << DiagnosticsFile << ".json"); diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index efe9231f5960..8da2e93e7d6d 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp @@ -231,10 +231,6 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { Cout << Endl << "Statistics:" << Endl << *stats; } - if (meta) { - Cout << Endl << "Meta:" << Endl << NJson::PrettifyJson(*meta, true) << Endl;; - } - if (plan) { if (!ExplainMode && !ExplainAnalyzeMode && (OutputFormat == EDataFormat::Default || OutputFormat == EDataFormat::Pretty)) { From 1eaad6c0455be8f4cd991c05bb0e10023b656aee Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 14 Feb 2025 11:42:54 +0300 Subject: [PATCH 19/22] Fixes --- ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp | 4 ++-- ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index ff164130d9ea..43424f36562c 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -532,7 +532,7 @@ void TCommandExecuteQuery::PrintDataQueryResponse(NTable::TDataQueryResult& resu } if (!DiagnosticsFile.empty()) { - TFileOutput file(DiagnosticsFile + ".json"); + TFileOutput file(DiagnosticsFile); NJson::TJsonValue diagnosticsJson(NJson::JSON_MAP); @@ -817,7 +817,7 @@ bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { } if (!DiagnosticsFile.empty()) { - TFileOutput file(TStringBuilder() << DiagnosticsFile << ".json"); + TFileOutput file(DiagnosticsFile); NJson::TJsonValue diagnosticsJson(NJson::JSON_MAP); diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index 8da2e93e7d6d..5ca0e0a7e988 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp @@ -245,7 +245,7 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { } if (!DiagnosticsFile.empty()) { - TFileOutput file(TStringBuilder() << DiagnosticsFile << ".json"); + TFileOutput file(DiagnosticsFile); NJson::TJsonValue diagnosticsJson(NJson::JSON_MAP); From 95530047c84ec2548238f8695c63487ac358ed02 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 14 Feb 2025 17:17:08 +0300 Subject: [PATCH 20/22] Fixes --- ydb/core/kqp/ut/query/kqp_query_ut.cpp | 13 ++++++++++--- .../cpp/include/ydb-cpp-sdk/client/table/table.h | 14 ++++++++++---- ydb/public/sdk/cpp/src/client/table/table.cpp | 8 -------- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/ydb/core/kqp/ut/query/kqp_query_ut.cpp b/ydb/core/kqp/ut/query/kqp_query_ut.cpp index 5cccc5c3221b..4549a493ad7f 100644 --- a/ydb/core/kqp/ut/query/kqp_query_ut.cpp +++ b/ydb/core/kqp/ut/query/kqp_query_ut.cpp @@ -211,10 +211,13 @@ Y_UNIT_TEST_SUITE(KqpQuery) { UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); - UNIT_ASSERT_C(!result.GetMeta().empty(), "Query result meta is empty"); + auto stats = result.GetStats(); + UNIT_ASSERT(stats.has_value()); + + UNIT_ASSERT_C(stats->GetMeta().has_value(), "Query result meta is empty"); TStringStream in; - in << result.GetMeta(); + in << stats->GetMeta().value(); NJson::TJsonValue value; ReadJsonTree(&in, &value); @@ -234,12 +237,16 @@ Y_UNIT_TEST_SUITE(KqpQuery) { { auto settings = TExecDataQuerySettings(); + settings.CollectQueryStats(ECollectQueryStatsMode::Basic); auto result = session.ExecuteDataQuery(query, TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync(); UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString().c_str()); - UNIT_ASSERT_C(result.GetMeta().empty(), "Query result meta should be empty, but it's not"); + auto stats = result.GetStats(); + UNIT_ASSERT(stats.has_value()); + + UNIT_ASSERT_C(!stats->GetMeta().has_value(), "Query result meta should be empty, but it's not"); } } } diff --git a/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h b/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h index 0f94a89eae71..9f4da9974c0c 100644 --- a/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h +++ b/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h @@ -1163,8 +1163,10 @@ struct TStreamExecScanQuerySettings : public TRequestSettings= ECollectQueryStatsMode::Full to get QueryMeta in QueryStats + // Collect full query compilation diagnostics + FLUENT_SETTING_DEFAULT(bool, CollectFullDiagnostics, false); }; enum class EDataFormat { @@ -2036,8 +2038,6 @@ class TDataQueryResult : public TStatus { const std::string GetQueryPlan() const; - const std::string GetMeta() const; - private: std::optional Transaction_; std::vector ResultSets_; @@ -2110,6 +2110,11 @@ class TScanQueryPart : public TStreamPartStatus { const TQueryStats& GetQueryStats() const { return *QueryStats_; } TQueryStats ExtractQueryStats() { return std::move(*QueryStats_); } + // Deprecated. Use GetMeta() of TQueryStats + bool HasDiagnostics() const { return FakeDiagnostics_.has_value(); } + const std::string& GetDiagnostics() const { return *FakeDiagnostics_; } + std::string&& ExtractDiagnostics() { return std::move(*FakeDiagnostics_); } + TScanQueryPart(TStatus&& status) : TStreamPartStatus(std::move(status)) {} @@ -2128,6 +2133,7 @@ class TScanQueryPart : public TStreamPartStatus { private: std::optional ResultSet_; std::optional QueryStats_; + std::optional FakeDiagnostics_; }; using TAsyncScanQueryPart = NThreading::TFuture; diff --git a/ydb/public/sdk/cpp/src/client/table/table.cpp b/ydb/public/sdk/cpp/src/client/table/table.cpp index 5a41c6d6f877..4ebdbdd794d9 100644 --- a/ydb/public/sdk/cpp/src/client/table/table.cpp +++ b/ydb/public/sdk/cpp/src/client/table/table.cpp @@ -2220,14 +2220,6 @@ const std::string TDataQueryResult::GetQueryPlan() const { } } -const std::string TDataQueryResult::GetMeta() const { - if (QueryStats_.has_value()) { - return NYdb::TProtoAccessor::GetProto(*QueryStats_).query_meta(); - } else { - return ""; - } -} - //////////////////////////////////////////////////////////////////////////////// TBeginTransactionResult::TBeginTransactionResult(TStatus&& status, TTransaction transaction) From 22df0b804bf3ac91972f8baaae742e25728c2fef Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 14 Feb 2025 17:24:56 +0300 Subject: [PATCH 21/22] Update ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h Co-authored-by: Bulat --- ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h b/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h index 9f4da9974c0c..da75c00d0c28 100644 --- a/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h +++ b/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h @@ -1164,7 +1164,7 @@ struct TStreamExecScanQuerySettings : public TRequestSettings= ECollectQueryStatsMode::Full to get QueryMeta in QueryStats + // Deprecated. Use CollectQueryStats >= ECollectQueryStatsMode::Full to get QueryMeta in QueryStats // Collect full query compilation diagnostics FLUENT_SETTING_DEFAULT(bool, CollectFullDiagnostics, false); }; From 010347ef8fbb7414ee936d3cf3d830a55fe72f46 Mon Sep 17 00:00:00 2001 From: Nikolay Shumkov Date: Fri, 14 Feb 2025 17:41:45 +0300 Subject: [PATCH 22/22] Fixes --- .../cpp/include/ydb-cpp-sdk/client/table/table.h | 14 ++++++++------ .../sdk/cpp/src/client/table/impl/readers.cpp | 7 +++++-- .../sdk/cpp/src/client/table/impl/table_client.cpp | 1 + 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h b/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h index 9f4da9974c0c..d15c1cd1049a 100644 --- a/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h +++ b/ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/table/table.h @@ -2111,29 +2111,31 @@ class TScanQueryPart : public TStreamPartStatus { TQueryStats ExtractQueryStats() { return std::move(*QueryStats_); } // Deprecated. Use GetMeta() of TQueryStats - bool HasDiagnostics() const { return FakeDiagnostics_.has_value(); } - const std::string& GetDiagnostics() const { return *FakeDiagnostics_; } - std::string&& ExtractDiagnostics() { return std::move(*FakeDiagnostics_); } + bool HasDiagnostics() const { return Diagnostics_.has_value(); } + const std::string& GetDiagnostics() const { return *Diagnostics_; } + std::string&& ExtractDiagnostics() { return std::move(*Diagnostics_); } TScanQueryPart(TStatus&& status) : TStreamPartStatus(std::move(status)) {} - TScanQueryPart(TStatus&& status, const std::optional& queryStats) + TScanQueryPart(TStatus&& status, const std::optional& queryStats, const std::optional& diagnostics) : TStreamPartStatus(std::move(status)) , QueryStats_(queryStats) + , Diagnostics_(diagnostics) {} - TScanQueryPart(TStatus&& status, TResultSet&& resultSet, const std::optional& queryStats) + TScanQueryPart(TStatus&& status, TResultSet&& resultSet, const std::optional& queryStats, const std::optional& diagnostics) : TStreamPartStatus(std::move(status)) , ResultSet_(std::move(resultSet)) , QueryStats_(queryStats) + , Diagnostics_(diagnostics) {} private: std::optional ResultSet_; std::optional QueryStats_; - std::optional FakeDiagnostics_; + std::optional Diagnostics_; }; using TAsyncScanQueryPart = NThreading::TFuture; diff --git a/ydb/public/sdk/cpp/src/client/table/impl/readers.cpp b/ydb/public/sdk/cpp/src/client/table/impl/readers.cpp index bc4ee7a5e6f7..acb5f6dd58b1 100644 --- a/ydb/public/sdk/cpp/src/client/table/impl/readers.cpp +++ b/ydb/public/sdk/cpp/src/client/table/impl/readers.cpp @@ -82,16 +82,19 @@ TAsyncScanQueryPart TScanQueryPartIterator::TReaderImpl::ReadNext(std::shared_pt TPlainStatus plainStatus{clientStatus, std::move(issues), self->Endpoint_, {}}; TStatus status{std::move(plainStatus)}; std::optional queryStats; + std::optional diagnostics; if (self->Response_.result().has_query_stats()) { queryStats = TQueryStats(self->Response_.result().query_stats()); } + diagnostics = self->Response_.result().query_full_diagnostics(); + if (self->Response_.result().has_result_set()) { promise.SetValue({std::move(status), - TResultSet(std::move(*self->Response_.mutable_result()->mutable_result_set())), queryStats}); + TResultSet(std::move(*self->Response_.mutable_result()->mutable_result_set())), queryStats, diagnostics}); } else { - promise.SetValue({std::move(status), queryStats}); + promise.SetValue({std::move(status), queryStats, diagnostics}); } } }; diff --git a/ydb/public/sdk/cpp/src/client/table/impl/table_client.cpp b/ydb/public/sdk/cpp/src/client/table/impl/table_client.cpp index 8441f02581ef..af68960e836a 100644 --- a/ydb/public/sdk/cpp/src/client/table/impl/table_client.cpp +++ b/ydb/public/sdk/cpp/src/client/table/impl/table_client.cpp @@ -1073,6 +1073,7 @@ TFuture> TT } request.set_collect_stats(GetStatsCollectionMode(settings.CollectQueryStats_)); + request.set_collect_full_diagnostics(settings.CollectFullDiagnostics_); auto promise = NewPromise>();