not null constraint fix for column addition (#1137)

* not null constraint fix for column addition

* fixing tests according to the changes

* fix scheme tests

ydb/core/kqp/provider/yql_kikimr_exec.cpp

@@ -1086,6 +1086,9 @@ public:
                         auto dataType = actualType->Cast<TDataExprType>();
                         SetColumnType(*add_column->mutable_type(), TString(dataType->GetName()), notNull);
 
+                        ::NKikimrIndexBuilder::TColumnBuildSetting* columnBuild = nullptr;
+                        bool hasDefaultValue = false;
+                        bool hasNotNull = false;
                         if (columnTuple.Size() > 2) {
                             auto columnConstraints = columnTuple.Item(2).Cast<TCoNameValueTuple>();
                             for(const auto& constraint: columnConstraints.Value().Cast<TCoNameValueTupleList>()) {
@@ -1094,13 +1097,32 @@ public:
                                         "Column addition with serial data type is unsupported"));
                                     return SyncError();
                                 } else if (constraint.Name().Value() == "default") {
-                                    auto columnBuild = indexBuildSettings.mutable_column_build_operation()->add_column();
+                                    if (columnBuild == nullptr) {
+                                        columnBuild = indexBuildSettings.mutable_column_build_operation()->add_column();
+                                    }
+
                                     columnBuild->SetColumnName(TString(columnName));
                                     FillLiteralProto(constraint.Value().Cast<TCoDataCtor>(), *columnBuild->mutable_default_from_literal());
+                                    hasDefaultValue = true;
+                                } else if (constraint.Name().Value() == "not_null") {
+                                    if (columnBuild == nullptr) {
+                                        columnBuild = indexBuildSettings.mutable_column_build_operation()->add_column();
+                                    }
+
+                                    columnBuild->SetNotNull(true);
+                                    hasNotNull = true;
                                 }
                             }
                         }
 
+                        if (hasNotNull && !hasDefaultValue) {
+                            ctx.AddError(
+                                YqlIssue(ctx.GetPosition(columnTuple.Pos()),
+                                    TIssuesIds::KIKIMR_BAD_REQUEST,
+                                    "Cannot add not null column without default value"));
+                            return SyncError();
+                        }
+
                         if (columnTuple.Size() > 3) {
                             auto families = columnTuple.Item(3).Cast<TCoAtomList>();
                             if (families.Size() > 1) {
@@ -1112,6 +1134,12 @@ public:
                             for (auto family : families) {
                                 add_column->set_family(TString(family.Value()));
                             }
+
+                            if (columnBuild) {
+                                for (auto family : families) {
+                                    columnBuild->SetFamily(TString(family.Value()));
+                                }
+                            }
                         }
                     }
                 } else if (name == "dropColumns") {

ydb/core/kqp/ut/scheme/kqp_constraints_ut.cpp

@@ -291,6 +291,7 @@ Y_UNIT_TEST_SUITE(KqpConstraints) {
     Y_UNIT_TEST(AlterTableAddColumnWithDefaultValue) {
         NKikimrConfig::TAppConfig appConfig;
         appConfig.MutableTableServiceConfig()->SetEnableSequences(false);
+        appConfig.MutableFeatureFlags()->SetEnableAddColumsWithDefaults(true);
         auto serverSettings = TKikimrSettings().SetAppConfig(appConfig);
         TKikimrRunner kikimr(serverSettings);
         auto db = kikimr.GetTableClient();
@@ -588,6 +589,8 @@ Y_UNIT_TEST_SUITE(KqpConstraints) {
         appConfig.MutableTableServiceConfig()->SetEnableSequences(false);
         appConfig.MutableTableServiceConfig()->SetEnableColumnsWithDefault(true);
 
+        appConfig.MutableFeatureFlags()->SetEnableAddColumsWithDefaults(true);
+
         TKikimrRunner kikimr(TKikimrSettings().SetPQConfig(DefaultPQConfig()).SetAppConfig(appConfig));
         auto db = kikimr.GetTableClient();
         auto session = db.CreateSession().GetValueSync().GetSession();
@@ -661,7 +664,7 @@ Y_UNIT_TEST_SUITE(KqpConstraints) {
 
         fCompareTable(R"(
             [
-                [[1u];["Old"];[1]];[[2u];["New"];[1]]
+                [[1u];["Old"];1];[[2u];["New"];1]
             ]
         )");
 
@@ -672,7 +675,7 @@ Y_UNIT_TEST_SUITE(KqpConstraints) {
 
         fCompareTable(R"(
             [
-                [[1u];["Old"];[1]];[[2u];["New"];[2]]
+                [[1u];["Old"];1];[[2u];["New"];2]
             ]
         )");
 
@@ -686,7 +689,7 @@ Y_UNIT_TEST_SUITE(KqpConstraints) {
 
         fCompareTable(R"(
             [
-                [[1u];["Old"];[1]];[[2u];["OldNew"];[2]];[[3u];["BrandNew"];[1]]
+                [[1u];["Old"];1];[[2u];["OldNew"];2];[[3u];["BrandNew"];1]
             ]
         )");
 

ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp

@@ -5553,7 +5553,7 @@ Y_UNIT_TEST_SUITE(KqpOlapScheme) {
         {
             auto alterQuery = TStringBuilder() << "ALTER TABLE `" << testTable.GetName() << "`ADD COLUMN new_column Uint64 NOT NULL;";
             auto alterResult = testHelper.GetSession().ExecuteSchemeQuery(alterQuery).GetValueSync();
-            UNIT_ASSERT_VALUES_EQUAL_C(alterResult.GetStatus(), EStatus::SCHEME_ERROR, alterResult.GetIssues().ToString());
+            UNIT_ASSERT_VALUES_EQUAL_C(alterResult.GetStatus(), EStatus::BAD_REQUEST, alterResult.GetIssues().ToString());
         }
     }
 

ydb/core/protos/feature_flags.proto

@@ -127,4 +127,5 @@ message TFeatureFlags {
     optional bool EnableViews = 112 [default = false];
     optional bool EnableServerlessExclusiveDynamicNodes = 113 [default = false];
     optional bool EnableAccessServiceBulkAuthorization = 114 [default = false];
+    optional bool EnableAddColumsWithDefaults = 115 [ default = false];
 }

ydb/core/protos/index_builder.proto

@@ -12,6 +12,8 @@ option java_package = "ru.yandex.kikimr.proto";
 message TColumnBuildSetting {
     optional string ColumnName = 1;
     optional Ydb.TypedValue default_from_literal = 2;
+    optional bool NotNull = 3;
+    optional string Family = 4;
 }
 
 message TColumnBuildSettings {

ydb/core/testlib/basics/feature_flags.h

@@ -56,6 +56,7 @@ public:
     FEATURE_FLAG_SETTER(EnableTablePgTypes)
     FEATURE_FLAG_SETTER(EnableServerlessExclusiveDynamicNodes)
     FEATURE_FLAG_SETTER(EnableAccessServiceBulkAuthorization)
+    FEATURE_FLAG_SETTER(EnableAddColumsWithDefaults)
 
     #undef FEATURE_FLAG_SETTER
 };

ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp

@@ -87,11 +87,19 @@ TTableInfo::TAlterDataPtr ParseParams(const TPath& path, TTableInfo::TPtr table,
 
     // Ignore column ids if they were passed by user!
     for (auto& col : *copyAlter.MutableColumns()) {
-        if (col.GetNotNull()) {
-            errStr = Sprintf("Not null columns is not supported for alter command");
+        bool hasDefault = col.HasDefaultFromLiteral();
+        if (hasDefault && !context.SS->EnableAddColumsWithDefaults) {
+            errStr = Sprintf("Column addition with default value is not supported now.");
             status = NKikimrScheme::StatusInvalidParameter;
             return nullptr;
         }
+
+        if (col.GetNotNull() && !hasDefault) {
+            errStr = Sprintf("Not null columns without defaults are not supported.");
+            status = NKikimrScheme::StatusInvalidParameter;
+            return nullptr;
+        }
+
         col.ClearId();
     }
 

ydb/core/tx/schemeshard/schemeshard_build_index.cpp

@@ -68,7 +68,9 @@ void TSchemeShard::PersistCreateBuildIndex(NIceDb::TNiceDb& db, const TIndexBuil
         db.Table<Schema::BuildColumnOperationSettings>().Key(info->Id, i).Update(
             NIceDb::TUpdate<Schema::BuildColumnOperationSettings::ColumnName>(info->BuildColumns[i].ColumnName),
             NIceDb::TUpdate<Schema::BuildColumnOperationSettings::DefaultFromLiteral>(
-                TString(info->BuildColumns[i].DefaultFromLiteral.SerializeAsString()))
+                TString(info->BuildColumns[i].DefaultFromLiteral.SerializeAsString())),
+            NIceDb::TUpdate<Schema::BuildColumnOperationSettings::NotNull>(info->BuildColumns[i].NotNull),
+            NIceDb::TUpdate<Schema::BuildColumnOperationSettings::FamilyName>(info->BuildColumns[i].FamilyName)
         );
     }
 }

ydb/core/tx/schemeshard/schemeshard_build_index__create.cpp

@@ -161,8 +161,11 @@ public:
             buildInfo->BuildColumns.reserve(settings.column_build_operation().column_size());
             for(int i = 0; i < settings.column_build_operation().column_size(); i++) {
                 const auto& colInfo = settings.column_build_operation().column(i);
+                bool notNull = colInfo.HasNotNull() && colInfo.GetNotNull();
+                TString familyName = colInfo.HasFamily() ? colInfo.GetFamily() : "";
                 buildInfo->BuildColumns.push_back(
-                    TIndexBuildInfo::TColumnBuildInfo(colInfo.GetColumnName(), colInfo.default_from_literal()));
+                    TIndexBuildInfo::TColumnBuildInfo(
+                        colInfo.GetColumnName(), colInfo.default_from_literal(), notNull, familyName));
             }
         }
 

ydb/core/tx/schemeshard/schemeshard_build_index__progress.cpp

@@ -98,6 +98,15 @@ THolder<TEvSchemeShard::TEvModifySchemeTransaction> AlterMainTablePropose(
             col->SetType(NScheme::TypeName(typeInfo, typeMod));
             col->SetName(colInfo.ColumnName);
             col->MutableDefaultFromLiteral()->CopyFrom(colInfo.DefaultFromLiteral);
+
+            if (!colInfo.FamilyName.empty()) {
+                col->SetFamilyName(colInfo.FamilyName);
+            }
+
+            if (colInfo.NotNull) {
+                col->SetNotNull(colInfo.NotNull);
+            }
+
         }
 
     } else {