Skip to content

Commit

Permalink
Fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@shnikd
shnikd committed Feb 13, 2025
1 parent c706d98 commit 527da8f
Show file tree
Hide file tree
Showing 18 changed files with 192 additions and 113 deletions.
2 changes: 1 addition & 1 deletion ydb/core/grpc_services/query/rpc_execute_query.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ class TExecuteQueryRPC : public TActorBootstrapped<TExecuteQueryRPC> {
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());
}
}

Expand Down
6 changes: 4 additions & 2 deletions ydb/core/grpc_services/rpc_execute_data_query.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActor<TExecuteDataQueryRPC, TE
req->collect_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;
Expand All @@ -178,7 +178,9 @@ class TExecuteDataQueryRPC : public TRpcKqpRequestActor<TExecuteDataQueryRPC, TE
if (from.HasQueryStats()) {
FillQueryStats(*to->mutable_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;
}
}
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/grpc_services/rpc_stream_execute_scan_query.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ class TStreamExecuteScanQueryRPC : public TActorBootstrapped<TStreamExecuteScanQ
}

if (collectDiagnostics) {
response.mutable_result()->mutable_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);
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/kqp/compile_service/kqp_compile_actor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,6 @@ class TKqpCompileActor : public TActorBootstrapped<TKqpCompileActor> {

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) {
Expand Down Expand Up @@ -383,6 +382,7 @@ class TKqpCompileActor : public TActorBootstrapped<TKqpCompileActor> {
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);

Expand Down
16 changes: 7 additions & 9 deletions ydb/core/kqp/ut/olap/helpers/query_executor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@

namespace NKikimr::NKqp {

TVector<THashMap<TString, NYdb::TValue>> CollectRows(NYdb::NTable::TScanQueryPartIterator& it, NJson::TJsonValue* statInfo /*= nullptr*/, NJson::TJsonValue* diagnostics /*= nullptr*/) {
TVector<THashMap<TString, NYdb::TValue>> CollectRows(NYdb::NTable::TScanQueryPartIterator& it, NJson::TJsonValue* statInfo /*= nullptr*/, NJson::TJsonValue* meta /*= nullptr*/) {
TVector<THashMap<TString, NYdb::TValue>> 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();
Expand All @@ -28,12 +28,10 @@ TVector<THashMap<TString, NYdb::TValue>> CollectRows(NYdb::NTable::TScanQueryPar
if (plan && statInfo) {
UNIT_ASSERT(NJson::ReadJsonFastTree(*plan, statInfo));
}
}

if (streamPart.HasDiagnostics()) {
TString diagnosticsString = 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));
}
}

Expand Down Expand Up @@ -70,4 +68,4 @@ TVector<THashMap<TString, NYdb::TValue>> ExecuteScanQuery(NYdb::NTable::TTableCl
return rows;
}

}
}
41 changes: 20 additions & 21 deletions ydb/core/kqp/ut/olap/kqp_olap_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ Y_UNIT_TEST_SUITE(KqpOlap) {
}
}

Y_UNIT_TEST(SimpleQueryOlapDiagnostics) {
Y_UNIT_TEST(SimpleQueryOlapMeta) {
auto settings = TKikimrSettings()
.SetWithSampleTables(false);
TKikimrRunner kikimr(settings);
Expand All @@ -317,9 +317,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");
}

{
Expand All @@ -334,22 +334,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");
}
}

Expand Down Expand Up @@ -2699,7 +2698,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();
Expand Down
33 changes: 16 additions & 17 deletions ydb/core/kqp/ut/query/kqp_query_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Expand Down Expand Up @@ -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");
}

{
Expand All @@ -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");
}
}
}
Expand Down
72 changes: 35 additions & 37 deletions ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ Y_UNIT_TEST_SUITE(KqpQueryService) {
}
}

Y_UNIT_TEST(ExecuteCollectFullDiagnostics) {
Y_UNIT_TEST(ExecuteCollectMeta) {
auto kikimr = DefaultKikimrRunner();
auto db = kikimr.GetQueryClient();

Expand All @@ -283,29 +283,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");
}

{
Expand All @@ -315,15 +314,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();

Expand All @@ -347,30 +346,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");
}

{
Expand All @@ -393,11 +391,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");
}
}

Expand Down
4 changes: 3 additions & 1 deletion ydb/public/api/protos/ydb_query_stats.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Loading

0 comments on commit 527da8f

Please sign in to comment.