From 04d730d6697355ffff8e0afbef96a5d115cfdee2 Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Tue, 1 Apr 2025 16:57:34 +0300 Subject: Fix optional columns handling in read_rows rpc (#15850) --- ydb/core/grpc_services/rpc_read_rows.cpp | 49 ++++++++++++---- ydb/core/kqp/ut/opt/kqp_kv_ut.cpp | 84 +++++++++++++++++++++++----- ydb/core/statistics/service/service_impl.cpp | 5 +- 3 files changed, 112 insertions(+), 26 deletions(-) diff --git a/ydb/core/grpc_services/rpc_read_rows.cpp b/ydb/core/grpc_services/rpc_read_rows.cpp index 8dbebc6339f..bc6b1e642c4 100644 --- a/ydb/core/grpc_services/rpc_read_rows.cpp +++ b/ydb/core/grpc_services/rpc_read_rows.cpp @@ -607,7 +607,11 @@ public: for (const auto& colMeta : RequestedColumnsMeta) { const auto type = getTypeFromColMeta(colMeta); auto* col = resultSet->Addcolumns(); - *col->mutable_type() = NYdb::TProtoAccessor::GetProto(type); + if (colMeta.IsNotNullColumn || colMeta.Type.GetTypeId() == NScheme::NTypeIds::Pg) { // pg type in nullable itself + *col->mutable_type() = NYdb::TProtoAccessor::GetProto(type); + } else { + *col->mutable_type()->mutable_optional_type()->mutable_item() = NYdb::TProtoAccessor::GetProto(type); + } *col->mutable_name() = colMeta.Name; } @@ -637,18 +641,41 @@ public: } case NScheme::NTypeIds::Decimal: { using namespace NYql::NDecimal; - - const auto loHi = cell.AsValue>(); - Ydb::Value valueProto; - valueProto.set_low_128(loHi.first); - valueProto.set_high_128(loHi.second); - const NYdb::TDecimalValue decimal(valueProto, - {static_cast(colMeta.Type.GetDecimalType().GetPrecision()), static_cast(colMeta.Type.GetDecimalType().GetScale())}); - vb.Decimal(decimal); + + NYdb::TDecimalType decimalType{ + static_cast(colMeta.Type.GetDecimalType().GetPrecision()), + static_cast(colMeta.Type.GetDecimalType().GetScale()) + }; + + if (cell.IsNull()) { + vb.EmptyOptional(NYdb::TTypeBuilder().Decimal(decimalType).Build()); + } else { + const auto loHi = cell.AsValue>(); + Ydb::Value valueProto; + valueProto.set_low_128(loHi.first); + valueProto.set_high_128(loHi.second); + if (colMeta.IsNotNullColumn) { + vb.Decimal({valueProto, decimalType}); + } else { + vb.BeginOptional(); + vb.Decimal({valueProto, decimalType}); + vb.EndOptional(); + } + } break; } default: { - ProtoValueFromCell(vb, colMeta.Type, cell); + if (cell.IsNull()) { + vb.EmptyOptional((NYdb::EPrimitiveType)colMeta.Type.GetTypeId()); + } else { + if (colMeta.IsNotNullColumn) { + ProtoValueFromCell(vb, colMeta.Type, cell); + } else { + vb.BeginOptional(); + ProtoValueFromCell(vb, colMeta.Type, cell); + vb.EndOptional(); + } + } break; } } @@ -744,6 +771,7 @@ private: , Name(colInfo.Name) , Type(colInfo.PType) , PTypeMod(colInfo.PTypeMod) + , IsNotNullColumn(colInfo.IsNotNullColumn) { } @@ -751,6 +779,7 @@ private: TString Name; NScheme::TTypeInfo Type; TString PTypeMod; + bool IsNotNullColumn; }; TVector RequestedColumnsMeta; diff --git a/ydb/core/kqp/ut/opt/kqp_kv_ut.cpp b/ydb/core/kqp/ut/opt/kqp_kv_ut.cpp index b3da7006c6e..79dbff9ca41 100644 --- a/ydb/core/kqp/ut/opt/kqp_kv_ut.cpp +++ b/ydb/core/kqp/ut/opt/kqp_kv_ut.cpp @@ -152,11 +152,11 @@ Y_UNIT_TEST_SUITE(KqpKv) { auto res = FormatResultSetYson(selectResult.GetResultSet()); CompareYson(R"( [ - [1858343823u;0u;"abcde"]; - [1921763476782200957u;1u;"abcde"]; - [3843526951706058091u;2u;"abcde"]; - [5765290426629915225u;3u;"abcde"]; - [7687053901553772359u;4u;"abcde"] + [[1858343823u];[0u];["abcde"]]; + [[1921763476782200957u];[1u];["abcde"]]; + [[3843526951706058091u];[2u];["abcde"]]; + [[5765290426629915225u];[3u];["abcde"]]; + [[7687053901553772359u];[4u];["abcde"]] ] )", TString{res}); } @@ -263,11 +263,11 @@ Y_UNIT_TEST_SUITE(KqpKv) { UNIT_ASSERT_C(selectResult.IsSuccess(), selectResult.GetIssues().ToString()); auto res = FormatResultSetYson(selectResult.GetResultSet()); CompareYson(R"([ - [10u;0u;"abcde"]; - [11u;1u;"abcde"]; - [12u;2u;"abcde"]; - [13u;3u;"abcde"]; - [14u;4u;"abcde"] + [[10u];[0u];["abcde"]]; + [[11u];[1u];["abcde"]]; + [[12u];[2u];["abcde"]]; + [[13u];[3u];["abcde"]]; + [[14u];[4u];["abcde"]] ])", TString{res}); } { @@ -364,7 +364,7 @@ Y_UNIT_TEST_SUITE(KqpKv) { UNIT_ASSERT_C(selectResult.IsSuccess(), selectResult.GetIssues().ToString()); auto res = FormatResultSetYson(selectResult.GetResultSet()); - CompareYson(Sprintf("[[%du;%du]]", valueToReturn_1, valueToReturn_2), TString{res}); + CompareYson(Sprintf("[[[%du];[%du]]]", valueToReturn_1, valueToReturn_2), TString{res}); } Y_UNIT_TEST_TWIN(ReadRows_ExternalBlobs, UseExtBlobsPrecharge) { @@ -813,9 +813,9 @@ Y_UNIT_TEST_SUITE(KqpKv) { auto res = FormatResultSetYson(selectResult.GetResultSet()); CompareYson(R"( [ - ["0.123456789";"0.123456789";"0.123456789";"0.123456789";0u]; - ["1.123456789";"1000.123456789";"10.123456789";"1000000.123456789";1u]; - ["2.123456789";"2000.123456789";"20.123456789";"2000000.123456789";2u] + [["0.123456789"];["0.123456789"];["0.123456789"];["0.123456789"];[0u]]; + [["1.123456789"];["1000.123456789"];["10.123456789"];["1000000.123456789"];[1u]]; + [["2.123456789"];["2000.123456789"];["20.123456789"];["2000000.123456789"];[2u]] ] )", TString{res}); } @@ -833,10 +833,64 @@ Y_UNIT_TEST_SUITE(KqpKv) { auto selectResult = db.ReadRows("/Root/TestTable", keys.Build()).GetValueSync(); UNIT_ASSERT_C(selectResult.IsSuccess(), selectResult.GetIssues().ToString()); auto res = FormatResultSetYson(selectResult.GetResultSet()); - CompareYson(R"([["inf";"inf";"inf";"inf";999999999u];])", TString{res}); + CompareYson(R"([[["inf"];["inf"];["inf"];["inf"];[999999999u]];])", TString{res}); } } + Y_UNIT_TEST(ReadRows_Nulls) { + auto settings = TKikimrSettings() + .SetWithSampleTables(false); + auto kikimr = TKikimrRunner{settings}; + auto db = kikimr.GetTableClient(); + auto session = db.CreateSession().GetValueSync().GetSession(); + + auto schemeResult = session.ExecuteSchemeQuery(R"( + CREATE TABLE TestTable ( + Key Uint64, + Data Uint32, + Value Utf8, + PRIMARY KEY (Key) + ); + )").GetValueSync(); + UNIT_ASSERT_C(schemeResult.IsSuccess(), schemeResult.GetIssues().ToString()); + + NYdb::TValueBuilder rows; + rows.BeginList(); + for (size_t i = 0; i < 5; ++i) { + rows.AddListItem() + .BeginStruct() + .AddMember("Key").Uint64(i * 1921763474923857134ull + 1858343823) + .EndStruct(); + } + rows.EndList(); + + auto upsertResult = db.BulkUpsert("/Root/TestTable", rows.Build()).GetValueSync(); + UNIT_ASSERT_C(upsertResult.IsSuccess(), upsertResult.GetIssues().ToString()); + + NYdb::TValueBuilder keys; + keys.BeginList(); + for (size_t i = 0; i < 5; ++i) { + keys.AddListItem() + .BeginStruct() + .AddMember("Key").Uint64(i * 1921763474923857134ull + 1858343823) + .EndStruct(); + } + keys.EndList(); + auto selectResult = db.ReadRows("/Root/TestTable", keys.Build()).GetValueSync(); + Cerr << "IsSuccess(): " << selectResult.IsSuccess() << " GetStatus(): " << selectResult.GetStatus() << Endl; + UNIT_ASSERT_C(selectResult.IsSuccess(), selectResult.GetIssues().ToString()); + auto res = FormatResultSetYson(selectResult.GetResultSet()); + CompareYson(R"( + [ + [[1858343823u];#;#]; + [[1921763476782200957u];#;#]; + [[3843526951706058091u];#;#]; + [[5765290426629915225u];#;#]; + [[7687053901553772359u];#;#] + ] + )", TString{res}); + } + } diff --git a/ydb/core/statistics/service/service_impl.cpp b/ydb/core/statistics/service/service_impl.cpp index 573ea21c482..d439f5a02a8 100644 --- a/ydb/core/statistics/service/service_impl.cpp +++ b/ydb/core/statistics/service/service_impl.cpp @@ -695,7 +695,10 @@ private: while(parser.TryNextRow()) { auto& col = parser.ColumnParser("data"); - query_response->Data = col.GetString(); + // may be not optional from versions before fix of bug https://github.com/ydb-platform/ydb/issues/15701 + query_response->Data = col.GetKind() == NYdb::TTypeParser::ETypeKind::Optional + ? col.GetOptionalString() + : col.GetString(); } } else { SA_LOG_E("[TStatService::ReadRowsResponse] QueryId[ " -- cgit v1.3