Fix optional columns handling in read_rows rpc (#15850)

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<std::pair<ui64, i64>>();
-                        Ydb::Value valueProto;
-                        valueProto.set_low_128(loHi.first);
-                        valueProto.set_high_128(loHi.second);
-                        const NYdb::TDecimalValue decimal(valueProto, 
-                            {static_cast<ui8>(colMeta.Type.GetDecimalType().GetPrecision()), static_cast<ui8>(colMeta.Type.GetDecimalType().GetScale())});
-                        vb.Decimal(decimal);
+
+                        NYdb::TDecimalType decimalType{
+                            static_cast<ui8>(colMeta.Type.GetDecimalType().GetPrecision()),
+                            static_cast<ui8>(colMeta.Type.GetDecimalType().GetScale())
+                        };
+
+                        if (cell.IsNull()) {
+                            vb.EmptyOptional(NYdb::TTypeBuilder().Decimal(decimalType).Build());
+                        } else {
+                            const auto loHi = cell.AsValue<std::pair<ui64, i64>>();
+                            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<TColumnMeta> 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[ "