diff --git a/ydb/core/grpc_services/query/rpc_execute_query.cpp b/ydb/core/grpc_services/query/rpc_execute_query.cpp index 1730d30a78f0..10d38dfba818 100644 --- a/ydb/core/grpc_services/query/rpc_execute_query.cpp +++ b/ydb/core/grpc_services/query/rpc_execute_query.cpp @@ -175,6 +175,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() { @@ -278,6 +297,8 @@ class TExecuteQueryRPC : public TActorBootstrapped { settings, req->pool_id()); + ev->Record.MutableRequest()->SetCollectDiagnostics(NeedCollectDiagnostics(*req)); + if (!ctx.Send(NKqp::MakeKqpProxyID(ctx.SelfID.NodeId()), ev.Release(), 0, 0, Span_.GetTraceId())) { NYql::TIssues issues; issues.AddIssue(MakeIssue(NKikimrIssues::TIssuesIds::DEFAULT_ERROR, "Internal error")); @@ -375,6 +396,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) { diff --git a/ydb/core/grpc_services/rpc_execute_data_query.cpp b/ydb/core/grpc_services/rpc_execute_data_query.cpp index 6152ae5f14cb..473ab06e8e08 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; @@ -146,6 +156,8 @@ 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; @@ -166,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; } } 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..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()) { @@ -228,7 +249,7 @@ class TStreamExecuteScanQueryRPC : public TActorBootstrappedRecord.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; @@ -291,6 +312,7 @@ class TStreamExecuteScanQueryRPC : public TActorBootstrappedGetProtoRequest()); bool reportPlan = reportStats && NeedReportPlan(*Request_->GetProtoRequest()); + bool collectDiagnostics = NeedCollectDiagnostics(*Request_->GetProtoRequest()); if (reportStats) { if (kqpResponse.HasQueryStats()) { @@ -308,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 19aad90211f2..618aa74de9f0 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(); } 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 d072d9e642c0..ac64fec2056b 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; @@ -665,7 +669,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) { @@ -674,7 +678,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 01d32434aa7a..c3f1a4e5809e 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; @@ -464,7 +464,7 @@ class TKqpCompileService : public TActorBootstrapped { TKqpCompileSettings compileSettings( request.KeepInCache, - request.IsQueryActionPrepare, + request.QueryAction, request.PerStatementResult, request.Deadline, ev->Get()->Split @@ -532,7 +532,7 @@ class TKqpCompileService : public TActorBootstrapped { TKqpCompileSettings compileSettings( true, - request.IsQueryActionPrepare, + request.QueryAction, false, request.Deadline, ev->Get()->Split @@ -626,7 +626,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()) { @@ -849,7 +849,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.ExecutorThread.RegisterActor(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 1e70d62d8b18..1df7fab8e8d9 100644 --- a/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp +++ b/ydb/core/kqp/ut/olap/kqp_olap_ut.cpp @@ -308,7 +308,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` @@ -325,7 +325,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 14750787df5f..04e3f3fab959 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.CollectQueryStats(ECollectQueryStatsMode::Full); + + 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 bd3080432739..d5a8736efe0f 100644 --- a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp +++ b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp @@ -272,6 +272,135 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { } } + Y_UNIT_TEST(ExecuteCollectFullDiagnostics) { + auto kikimr = DefaultKikimrRunner(); + auto db = kikimr.GetQueryClient(); + + { + TExecuteQuerySettings settings; + settings.StatsMode(EStatsMode::Full); + + 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"); + + TStringStream in; + in << stats.query_diagnostics(); + 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 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"); + } + } + + 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"); + } + } + 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_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 172c97ceb3d0..0c2939e48cc8 100644 --- a/ydb/public/api/protos/ydb_table.proto +++ b/ydb/public/api/protos/ydb_table.proto @@ -1288,9 +1288,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 { @@ -1306,7 +1304,5 @@ 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 } 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 0200655e536a..53cf75945f18 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -732,6 +732,7 @@ template bool TCommandExecuteQuery::PrintQueryResponse(TIterator& result) { TMaybe stats; TMaybe fullStats; + TString diagnostics; { TResultSetPrinter printer(OutputFormat, &IsInterrupted); @@ -872,10 +873,6 @@ int TCommandExplain::Run(TConfig& config) { settings.Explain(true); } - if (CollectFullDiagnostics) { - settings.CollectFullDiagnostics(true); - } - auto result = client.StreamExecuteScanQuery(Query, settings).GetValueSync(); ThrowOnError(result); @@ -892,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_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index abfd8377149c..704577770162 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 @@ -40,6 +42,8 @@ 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(); @@ -183,6 +187,7 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { TMaybe stats; TMaybe plan; TMaybe ast; + TMaybe diagnostics; { TResultSetPrinter printer(OutputFormat, &IsInterrupted); @@ -204,6 +209,7 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { if (queryStats.GetPlan()) { plan = queryStats.GetPlan(); } + diagnostics = queryStats.GetDiagnostics(); } } } // TResultSetPrinter destructor should be called before printing stats @@ -222,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)) { @@ -235,6 +245,30 @@ int TCommandSql::PrintResponse(NQuery::TExecuteQueryIterator& result) { queryPlanPrinter.Print(*plan); } + 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()) { 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 19dccfd01e61..ce6eb2780808 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.h @@ -28,6 +28,7 @@ class TCommandSql : public TYdbCommand, public TCommandWithOutput, public TComma int PrintResponse(NQuery::TExecuteQueryIterator& result); TString CollectStatsMode; + TString DiagnosticsFile; TString Query; TString QueryFile; TString Syntax; 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 93b91c5ac2e1..3a2e597f3f6b 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 @@ -145,6 +145,7 @@ 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 c007547d4e84..c2dcdf09a2c6 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; @@ -56,6 +57,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 15c8a15a5134..6115214f1861 100644 --- a/ydb/public/sdk/cpp/client/ydb_query/stats.h +++ b/ydb/public/sdk/cpp/client/ydb_query/stats.h @@ -31,6 +31,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 11a8d4d0b176..721337dbde4b 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 38a84818661d..0449591a27e8 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/query_stats/stats.h b/ydb/public/sdk/cpp/client/ydb_table/query_stats/stats.h index 29a7c87fa1f8..fab414153512 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 03ddeb2990a8..a5217478be59 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.cpp +++ b/ydb/public/sdk/cpp/client/ydb_table/table.cpp @@ -2209,6 +2209,14 @@ const TString TDataQueryResult::GetQueryPlan() const { } } +const TString TDataQueryResult::GetDiagnostics() const { + if (QueryStats_.Defined()) { + return NYdb::TProtoAccessor::GetProto(*QueryStats_.Get()).query_diagnostics(); + } 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 ef0060886e9d..72bda2f9a22d 100644 --- a/ydb/public/sdk/cpp/client/ydb_table/table.h +++ b/ydb/public/sdk/cpp/client/ydb_table/table.h @@ -1164,9 +1164,6 @@ struct TStreamExecScanQuerySettings : public TRequestSettings Transaction_; TVector ResultSets_; @@ -2104,31 +2103,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;