From fe21f716ae5a0864c110c98c14109807b9ef9ac7 Mon Sep 17 00:00:00 2001 From: Vitalii Gridnev Date: Sat, 15 Nov 2025 15:13:47 +0300 Subject: [PATCH 1/2] implement the unit test --- ydb/core/kqp/ut/sysview/kqp_sys_view_ut.cpp | 54 +++++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/ydb/core/kqp/ut/sysview/kqp_sys_view_ut.cpp b/ydb/core/kqp/ut/sysview/kqp_sys_view_ut.cpp index 94ecb3540eb7..b2f58957f39f 100644 --- a/ydb/core/kqp/ut/sysview/kqp_sys_view_ut.cpp +++ b/ydb/core/kqp/ut/sysview/kqp_sys_view_ut.cpp @@ -564,6 +564,54 @@ order by SessionId;)", "%Y-%m-%d %H:%M:%S %Z", sessionsSet.front().GetId().data( CompareYson(expected, StreamResultToYson(it)); } + Y_UNIT_TEST(NodesOrderByDesc) { + // Test to reproduce issue #12585: ORDER BY DESC doesn't work for sys views + // The sys view actors ignore the direction flag and don't guarantee order + TKikimrRunner kikimr("", KikimrDefaultUtDomainRoot, 5); + auto client = kikimr.GetQueryClient(); + auto session = client.GetSession().GetValueSync().GetSession(); + + ui32 offset = kikimr.GetTestServer().GetRuntime()->GetNodeId(0); + + auto result = session.ExecuteQuery(R"(--!syntax_v1 + SELECT NodeId, Host + FROM `/Root/.sys/nodes` + ORDER BY NodeId DESC + )", NYdb::NQuery::TTxControl::NoTx()).GetValueSync(); + + UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); + + // Collect all results + TVector nodeIds; + auto resultSet = result.GetResultSet(0); + NYdb::TResultSetParser parser(resultSet); + while (parser.TryNextRow()) { + auto nodeId = parser.ColumnParser("NodeId").GetOptionalUint32().value(); + nodeIds.push_back(nodeId); + } + + // Verify we got all 5 nodes + UNIT_ASSERT_VALUES_EQUAL(nodeIds.size(), 5); + + // Verify results are in descending order (this should fail if the bug exists) + // According to issue #12585, sys view actors ignore the direction flag + // and don't guarantee order, so this assertion should fail + for (size_t i = 1; i < nodeIds.size(); ++i) { + UNIT_ASSERT_C(nodeIds[i - 1] >= nodeIds[i], + TStringBuilder() << "Results not in descending order: " + << "nodeIds[" << (i - 1) << "] = " << nodeIds[i - 1] + << " < nodeIds[" << i << "] = " << nodeIds[i] + << ". ORDER BY DESC is being ignored by sys view actors."); + } + + // Verify exact expected order: offset+4, offset+3, offset+2, offset+1, offset + UNIT_ASSERT_VALUES_EQUAL(nodeIds[0], offset + 4); + UNIT_ASSERT_VALUES_EQUAL(nodeIds[1], offset + 3); + UNIT_ASSERT_VALUES_EQUAL(nodeIds[2], offset + 2); + UNIT_ASSERT_VALUES_EQUAL(nodeIds[3], offset + 1); + UNIT_ASSERT_VALUES_EQUAL(nodeIds[4], offset); + } + Y_UNIT_TEST(QueryStatsSimple) { auto checkTable = [&] (const TStringBuf tableName) { TKikimrRunner kikimr("", KikimrDefaultUtDomainRoot, 3); @@ -920,15 +968,15 @@ order by SessionId;)", "%Y-%m-%d %H:%M:%S %Z", sessionsSet.front().GetId().data( ); )").ExtractValueSync(); UNIT_ASSERT_VALUES_EQUAL_C(createResult.GetStatus(), EStatus::SUCCESS, createResult.GetIssues()); - + auto insertResult = session.ExecuteDataQuery(R"( INSERT INTO TestTable (Key, Value) VALUES (42, "val"), (NULL, "val1"); )", TTxControl::BeginTx().CommitTx()).GetValueSync(); UNIT_ASSERT_C(insertResult.IsSuccess(), insertResult.GetIssues().ToString()); } - - + + auto session = db.CreateSession().GetValueSync().GetSession(); TString query = R"( SELECT * FROM TestTable WHERE Key in [42, NULL]; From a32e622fae9e0fb695a8d53fc8dd7846370e3c07 Mon Sep 17 00:00:00 2001 From: Vitalii Gridnev Date: Sat, 15 Nov 2025 15:41:37 +0300 Subject: [PATCH 2/2] don't remove DESC sorting for sys view reads --- .../kqp/opt/physical/kqp_opt_phy_sort.cpp | 14 ++ ydb/core/kqp/ut/sysview/kqp_sys_view_ut.cpp | 237 ++++++++++++++++++ 2 files changed, 251 insertions(+) diff --git a/ydb/core/kqp/opt/physical/kqp_opt_phy_sort.cpp b/ydb/core/kqp/opt/physical/kqp_opt_phy_sort.cpp index 21f8cfa4be7e..317c7344b03b 100644 --- a/ydb/core/kqp/opt/physical/kqp_opt_phy_sort.cpp +++ b/ydb/core/kqp/opt/physical/kqp_opt_phy_sort.cpp @@ -93,6 +93,13 @@ TExprBase KqpRemoveRedundantSortOverReadTable(TExprBase node, TExprContext& ctx, } if (direction == ESortDirection::Reverse) { + // For sys views, we need to set reverse flag even if UseSource returns false + // because sys view actors can handle reverse direction + bool isSysView = tableDesc.Metadata->Kind == EKikimrTableKind::SysView; + if (isSysView) { + return node; + } + if (!UseSource(kqpCtx, tableDesc) && kqpCtx.IsScanQuery()) { return node; } @@ -206,6 +213,13 @@ TExprBase KqpRemoveRedundantSortOverReadTableFSM( } if (isReversed) { + // For sys views, we need to set reverse flag even if UseSource returns false + // because sys view actors can handle reverse direction + bool isSysView = tableDesc.Metadata->Kind == EKikimrTableKind::SysView; + if (isSysView) { + return node; + } + if (!UseSource(kqpCtx, tableDesc) && kqpCtx.IsScanQuery()) { return node; } diff --git a/ydb/core/kqp/ut/sysview/kqp_sys_view_ut.cpp b/ydb/core/kqp/ut/sysview/kqp_sys_view_ut.cpp index b2f58957f39f..69b4fc764766 100644 --- a/ydb/core/kqp/ut/sysview/kqp_sys_view_ut.cpp +++ b/ydb/core/kqp/ut/sysview/kqp_sys_view_ut.cpp @@ -612,6 +612,243 @@ order by SessionId;)", "%Y-%m-%d %H:%M:%S %Z", sessionsSet.front().GetId().data( UNIT_ASSERT_VALUES_EQUAL(nodeIds[4], offset); } + Y_UNIT_TEST(PartitionStatsOrderByDesc) { + // Test ORDER BY DESC for partition_stats sys view + // Primary key: OwnerId, PathId, PartIdx, FollowerId + TKikimrRunner kikimr; + auto client = kikimr.GetQueryClient(); + auto session = client.GetSession().GetValueSync().GetSession(); + + auto result = session.ExecuteQuery(R"(--!syntax_v1 + SELECT OwnerId, PathId, PartIdx, FollowerId, Path + FROM `/Root/.sys/partition_stats` + ORDER BY OwnerId DESC, PathId DESC, PartIdx DESC, FollowerId DESC + )", NYdb::NQuery::TTxControl::NoTx()).GetValueSync(); + + UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); + + // Collect all results + struct TPartitionKey { + ui64 OwnerId; + ui64 PathId; + ui64 PartIdx; + ui32 FollowerId; + }; + TVector partitionKeys; + auto resultSet = result.GetResultSet(0); + NYdb::TResultSetParser parser(resultSet); + while (parser.TryNextRow()) { + auto ownerId = parser.ColumnParser("OwnerId").GetOptionalUint64().value(); + auto pathId = parser.ColumnParser("PathId").GetOptionalUint64().value(); + auto partIdx = parser.ColumnParser("PartIdx").GetOptionalUint64().value(); + auto followerId = parser.ColumnParser("FollowerId").GetOptionalUint32().value(); + partitionKeys.push_back({ownerId, pathId, partIdx, followerId}); + } + + // Verify we got some results + UNIT_ASSERT_C(partitionKeys.size() > 0, "Expected at least one partition"); + + // Verify results are in descending order by OwnerId, PathId, PartIdx, FollowerId + for (size_t i = 1; i < partitionKeys.size(); ++i) { + const auto& prev = partitionKeys[i - 1]; + const auto& curr = partitionKeys[i]; + + auto prevKey = std::tie(prev.OwnerId, prev.PathId, prev.PartIdx, prev.FollowerId); + auto currKey = std::tie(curr.OwnerId, curr.PathId, curr.PartIdx, curr.FollowerId); + + UNIT_ASSERT_C(prevKey >= currKey, + TStringBuilder() << "Results not in descending order: " + << "partitionKeys[" << (i - 1) << "] = (" << prev.OwnerId << ", " << prev.PathId << ", " << prev.PartIdx << ", " << prev.FollowerId << ")" + << " < partitionKeys[" << i << "] = (" << curr.OwnerId << ", " << curr.PathId << ", " << curr.PartIdx << ", " << curr.FollowerId << ")" + << ". ORDER BY DESC is being ignored by sys view actors."); + } + } + + Y_UNIT_TEST(QuerySessionsOrderByDesc) { + // Test ORDER BY DESC for query_sessions sys view + NKikimrConfig::TFeatureFlags featureFlags; + featureFlags.SetEnableRealSystemViewPaths(true); + TKikimrSettings settings; + settings.SetWithSampleTables(false); + settings.SetFeatureFlags(featureFlags); + settings.SetAuthToken("root@builtin"); + TKikimrRunner kikimr(settings); + + auto client = kikimr.GetQueryClient(); + auto session = client.GetSession().GetValueSync().GetSession(); + + // Create some sessions to have data + const size_t sessionsCount = 5; + std::vector sessionsSet; + for(ui32 i = 0; i < sessionsCount; i++) { + sessionsSet.emplace_back(std::move(client.GetSession().GetValueSync().GetSession())); + } + + // Wait a bit for sessions to be registered + ::Sleep(TDuration::MilliSeconds(100)); + + auto result = session.ExecuteQuery(R"(--!syntax_v1 + SELECT SessionId, State, NodeId + FROM `/Root/.sys/query_sessions` + ORDER BY SessionId DESC + )", NYdb::NQuery::TTxControl::NoTx()).GetValueSync(); + + UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); + + // Collect all results + TVector sessionIds; + auto resultSet = result.GetResultSet(0); + NYdb::TResultSetParser parser(resultSet); + while (parser.TryNextRow()) { + auto sessionId = parser.ColumnParser("SessionId").GetOptionalUtf8().value(); + sessionIds.push_back(sessionId); + } + + // Verify we got some results + UNIT_ASSERT_C(sessionIds.size() > 0, "Expected at least one session"); + + // Verify results are in descending order (lexicographically) + for (size_t i = 1; i < sessionIds.size(); ++i) { + UNIT_ASSERT_C(sessionIds[i - 1] >= sessionIds[i], + TStringBuilder() << "Results not in descending order: " + << "sessionIds[" << (i - 1) << "] = \"" << sessionIds[i - 1] << "\"" + << " < sessionIds[" << i << "] = \"" << sessionIds[i] << "\"" + << ". ORDER BY DESC is being ignored by sys view actors."); + } + } + + Y_UNIT_TEST(CompileCacheQueriesOrderByDesc) { + // Test ORDER BY DESC for compile_cache_queries sys view + // Primary key: NodeId, QueryId + auto serverSettings = TKikimrSettings().SetKqpSettings({ NKikimrKqp::TKqpSetting() }); + serverSettings.AppConfig.MutableTableServiceConfig()->SetEnableImplicitQueryParameterTypes(true); + TKikimrRunner kikimr(serverSettings); + kikimr.GetTestServer().GetRuntime()->GetAppData().FeatureFlags.SetEnableCompileCacheView(true); + auto client = kikimr.GetQueryClient(); + auto session = client.GetSession().GetValueSync().GetSession(); + + // Execute some queries to populate compile cache + auto tableClient = kikimr.GetTableClient(); + for (ui32 i = 0; i < 3; ++i) { + auto tableSession = tableClient.CreateSession().GetValueSync().GetSession(); + auto paramsBuilder = TParamsBuilder(); + paramsBuilder.AddParam("$k").Uint64(i).Build(); + auto executedResult = tableSession.ExecuteDataQuery( + R"(DECLARE $k AS Uint64; + SELECT COUNT(*) FROM `/Root/EightShard` WHERE Key = $k;)", + TTxControl::BeginTx().CommitTx(), + paramsBuilder.Build() + ).GetValueSync(); + UNIT_ASSERT_C(executedResult.IsSuccess(), executedResult.GetIssues().ToString()); + } + + // Wait a bit for compile cache to be updated + ::Sleep(TDuration::MilliSeconds(100)); + + auto result = session.ExecuteQuery(R"(--!syntax_v1 + SELECT NodeId, QueryId, Query, CompilationDuration + FROM `/Root/.sys/compile_cache_queries` + ORDER BY NodeId DESC, QueryId DESC + )", NYdb::NQuery::TTxControl::NoTx()).GetValueSync(); + + UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); + + // Collect all results + struct TCompileCacheKey { + ui32 NodeId; + std::string QueryId; + }; + TVector compileCacheKeys; + auto resultSet = result.GetResultSet(0); + NYdb::TResultSetParser parser(resultSet); + while (parser.TryNextRow()) { + auto nodeId = parser.ColumnParser("NodeId").GetOptionalUint32().value(); + auto queryId = parser.ColumnParser("QueryId").GetOptionalUtf8().value(); + compileCacheKeys.push_back({nodeId, queryId}); + } + + // Verify we got some results + if (compileCacheKeys.size() > 0) { + // Verify results are in descending order by NodeId, then QueryId + for (size_t i = 1; i < compileCacheKeys.size(); ++i) { + const auto& prev = compileCacheKeys[i - 1]; + const auto& curr = compileCacheKeys[i]; + + auto prevKey = std::tie(prev.NodeId, prev.QueryId); + auto currKey = std::tie(curr.NodeId, curr.QueryId); + + UNIT_ASSERT_C(prevKey >= currKey, + TStringBuilder() << "Results not in descending order: " + << "compileCacheKeys[" << (i - 1) << "] = (" << prev.NodeId << ", \"" << prev.QueryId << "\")" + << " < compileCacheKeys[" << i << "] = (" << curr.NodeId << ", \"" << curr.QueryId << "\")" + << ". ORDER BY DESC is being ignored by sys view actors."); + } + } + } + + Y_UNIT_TEST(TopQueriesOrderByDesc) { + // Test ORDER BY DESC for top_queries sys views + TKikimrRunner kikimr("", KikimrDefaultUtDomainRoot, 3); + auto client = kikimr.GetQueryClient(); + auto session = client.GetSession().GetValueSync().GetSession(); + + // Execute some queries to populate stats + auto tableClient = kikimr.GetTableClient(); + auto tableSession = tableClient.CreateSession().GetValueSync().GetSession(); + { + auto result = tableSession.ExecuteDataQuery(Q_(R"( + SELECT * FROM `/Root/TwoShard` + )"), TTxControl::BeginTx().CommitTx()).GetValueSync(); + UNIT_ASSERT_VALUES_EQUAL(result.GetStatus(), EStatus::SUCCESS); + } + { + auto result = tableSession.ExecuteDataQuery(Q_(R"( + SELECT * FROM `/Root/EightShard` + )"), TTxControl::BeginTx().CommitTx()).GetValueSync(); + UNIT_ASSERT_VALUES_EQUAL(result.GetStatus(), EStatus::SUCCESS); + } + + // Wait a bit for stats to be collected + ::Sleep(TDuration::MilliSeconds(500)); + + // Test top_queries_by_read_bytes_one_minute + auto result = session.ExecuteQuery(R"(--!syntax_v1 + SELECT IntervalEnd, Rank, ReadBytes + FROM `/Root/.sys/top_queries_by_read_bytes_one_minute` + ORDER BY IntervalEnd DESC, Rank DESC + )", NYdb::NQuery::TTxControl::NoTx()).GetValueSync(); + + UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString()); + + // Collect all results + TVector> intervalEndRank; + auto resultSet = result.GetResultSet(0); + NYdb::TResultSetParser parser(resultSet); + while (parser.TryNextRow()) { + auto intervalEnd = parser.ColumnParser("IntervalEnd").GetOptionalTimestamp().value(); + auto rank = parser.ColumnParser("Rank").GetOptionalUint32().value(); + intervalEndRank.push_back({intervalEnd, rank}); + } + + // Verify we got some results (may be empty if no stats collected yet) + if (intervalEndRank.size() > 0) { + // Verify results are in descending order by IntervalEnd, then Rank + for (size_t i = 1; i < intervalEndRank.size(); ++i) { + const auto& prev = intervalEndRank[i - 1]; + const auto& curr = intervalEndRank[i]; + + auto prevKey = std::tie(prev.first, prev.second); + auto currKey = std::tie(curr.first, curr.second); + + UNIT_ASSERT_C(prevKey >= currKey, + TStringBuilder() << "Results not in descending order: " + << "intervalEndRank[" << (i - 1) << "] = (" << prev.first.ToString() << ", " << prev.second << ")" + << " < intervalEndRank[" << i << "] = (" << curr.first.ToString() << ", " << curr.second << ")" + << ". ORDER BY DESC is being ignored by sys view actors."); + } + } + } + Y_UNIT_TEST(QueryStatsSimple) { auto checkTable = [&] (const TStringBuf tableName) { TKikimrRunner kikimr("", KikimrDefaultUtDomainRoot, 3);