Fix alter table for composite marks mode

Кейс Леши описан в новом тесте. При alter затирался флаг композитных засечек и применение схемы падало в колумншарде.

Я починила кейс Леши, но у меня в процессе возник вопрос: это ожидаемое поведение, что мы при включении флага ForceCompositeMarks при перезагрузке таблетки всем старым снапшотам автоматом проставим CompositeMarks = true?

Что пофиксила  в итоге:

1)  У всех новых standalone-таблиц новых и tablestorов выставляется в схеме признак наличия композитных засечек. Алтеры на них идут так же со включенными засечками. На ранее созданные standalone-таблиц (и tablestor-ы) все апдейты идут без композитных засечек, т.е они не включаются на старых данных ничего не ломается. 
2) При выставлении ForceCompositeMarks, как мне пояснил snaury, ожидается, что произойдёт конвертация всех старых таблиц. поэтому мы и "перетираем" признак, записанный в схеме. Я добавила такую же логику в схемашарде, чтобы у нас приходили корректные алтеры после включения флага.
3) Добавила проверки совместимости схемы на Propose схемы
 4) Унесла парсинг CompositeIndexKey в IndexInfo к парсингу протобуфа схемы, потому что он парится из протобуфа схемы

11 files changed, 82 insertions, 23 deletions

diff --git a/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp b/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp
index 99c172db78b..b4b7a52c38f 100644
--- a/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp
+++ b/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp
@@ -4903,6 +4903,31 @@ namespace {
 
 Y_UNIT_TEST_SUITE(KqpOlapScheme) {
 
+    Y_UNIT_TEST(AddColumnLongPk) {
+        TKikimrSettings runnerSettings;
+        runnerSettings.WithSampleTables = false;
+        runnerSettings.FeatureFlags.SetForceColumnTablesCompositeMarks(false);
+        TTestHelper testHelper(runnerSettings);
+
+        TVector<TTestHelper::TColumnSchema> schema = {
+            TTestHelper::TColumnSchema().SetName("id").SetType(NScheme::NTypeIds::Int32).SetNullable(false),
+            TTestHelper::TColumnSchema().SetName("id_second").SetType(NScheme::NTypeIds::Int32).SetNullable(false),
+            TTestHelper::TColumnSchema().SetName("resource_id").SetType(NScheme::NTypeIds::Utf8),
+            TTestHelper::TColumnSchema().SetName("level").SetType(NScheme::NTypeIds::Int32)
+        };
+        TTestHelper::TColumnTable testTable;
+
+        testTable.SetName("/Root/ColumnTableTest").SetPrimaryKey({"id", "id_second"}).SetSharding({"id"}).SetSchema(schema);
+        testHelper.CreateTable(testTable);
+
+        {
+            schema.push_back(TTestHelper::TColumnSchema().SetName("new_column").SetType(NScheme::NTypeIds::Uint64));
+            auto alterQuery = TStringBuilder() << "ALTER TABLE `" << testTable.GetName() << "` ADD COLUMN new_column Uint64;";
+            auto alterResult = testHelper.GetSession().ExecuteSchemeQuery(alterQuery).GetValueSync();
+            UNIT_ASSERT_VALUES_EQUAL_C(alterResult.GetStatus(), EStatus::SUCCESS, alterResult.GetIssues().ToString());
+        }
+    }
+
     Y_UNIT_TEST(AddColumn) {
         TKikimrSettings runnerSettings;
         runnerSettings.WithSampleTables = false;
diff --git a/ydb/core/protos/flat_scheme_op.proto b/ydb/core/protos/flat_scheme_op.proto
index a7c6196a0a4..30da267d7fb 100644
--- a/ydb/core/protos/flat_scheme_op.proto
+++ b/ydb/core/protos/flat_scheme_op.proto
@@ -449,7 +449,7 @@ message TColumnTableSchema {
     //optional int32 DefaultCompressionLevel = 7; // deprecated, not used before replace
     optional TCompressionOptions DefaultCompression = 8;
 
-    optional bool CompositeMarks = 9;
+    optional bool CompositeMarks = 9 [ default = false ];
 }
 
 message TAlterColumnTableSchema {
diff --git a/ydb/core/tx/columnshard/columnshard__propose_transaction.cpp b/ydb/core/tx/columnshard/columnshard__propose_transaction.cpp
index c6a9274b20c..8f36356401a 100644
--- a/ydb/core/tx/columnshard/columnshard__propose_transaction.cpp
+++ b/ydb/core/tx/columnshard/columnshard__propose_transaction.cpp
@@ -51,13 +51,20 @@ bool TTxProposeTransaction::Execute(TTransactionContext& txc, const TActorContex
             if (!meta.Body.ParseFromString(txBody)) {
                 statusMessage = TStringBuilder()
                     << "Schema TxId# " << txId << " cannot be parsed";
+                status = NKikimrTxColumnShard::EResultStatus::SCHEMA_ERROR;
                 break;
             }
 
+            NOlap::ISnapshotSchema::TPtr currentSchema;
+            if (Self->TablesManager.HasPrimaryIndex()) {
+                currentSchema = Self->TablesManager.GetPrimaryIndexSafe().GetVersionedIndex().GetLastSchema();
+            }
+
             // Invalid body generated at a newer SchemeShard
-            if (!meta.Validate()) {
+            if (!meta.Validate(currentSchema)) {
                 statusMessage = TStringBuilder()
                     << "Schema TxId# " << txId << " cannot be proposed";
+                status = NKikimrTxColumnShard::EResultStatus::SCHEMA_ERROR;
                 break;
             }
 
diff --git a/ydb/core/tx/columnshard/columnshard_impl.cpp b/ydb/core/tx/columnshard/columnshard_impl.cpp
index 8dfe7f7b5f0..44f3a0bfde1 100644
--- a/ydb/core/tx/columnshard/columnshard_impl.cpp
+++ b/ydb/core/tx/columnshard/columnshard_impl.cpp
@@ -83,7 +83,7 @@ bool ValidateTablePreset(const NKikimrSchemeOp::TColumnTableSchemaPreset& preset
 
 }
 
-bool TColumnShard::TAlterMeta::Validate() const {
+bool TColumnShard::TAlterMeta::Validate(const NOlap::ISnapshotSchema::TPtr& schema) const {
     switch (Body.TxBody_case()) {
         case NKikimrTxColumnShard::TSchemaTxBody::kInitShard:
             break;
@@ -99,8 +99,25 @@ bool TColumnShard::TAlterMeta::Validate() const {
             }
             break;
         case NKikimrTxColumnShard::TSchemaTxBody::kAlterTable:
-        case NKikimrTxColumnShard::TSchemaTxBody::kDropTable:
+            if (schema) {
+                if (Body.HasAlterTable() && Body.GetAlterTable().HasSchema()) {
+                    return schema->GetIndexInfo().CheckAlterScheme(Body.GetAlterTable().GetSchema());
+                }
+                if (Body.HasAlterTable() && Body.GetAlterTable().HasSchemaPreset() && Body.GetAlterTable().GetSchemaPreset().HasSchema()) {
+                    return schema->GetIndexInfo().CheckAlterScheme(Body.GetAlterTable().GetSchemaPreset().GetSchema());
+                }
+            }
+            return true;
         case NKikimrTxColumnShard::TSchemaTxBody::kAlterStore:
+            if (schema) {
+                for (const auto& presetProto : Body.GetAlterStore().GetSchemaPresets()) {
+                    if (!schema->GetIndexInfo().CheckAlterScheme(presetProto.GetSchema())) {
+                        return false;
+                    }
+                }
+                return true;
+            }
+        case NKikimrTxColumnShard::TSchemaTxBody::kDropTable:
         case NKikimrTxColumnShard::TSchemaTxBody::TXBODY_NOT_SET:
             break;
     }
diff --git a/ydb/core/tx/columnshard/columnshard_impl.h b/ydb/core/tx/columnshard/columnshard_impl.h
index b48a44eca98..7e345880ca7 100644
--- a/ydb/core/tx/columnshard/columnshard_impl.h
+++ b/ydb/core/tx/columnshard/columnshard_impl.h
@@ -316,7 +316,7 @@ private:
         NKikimrTxColumnShard::TSchemaTxBody Body;
         THashSet<TActorId> NotifySubscribers;
 
-        bool Validate() const;
+        bool Validate(const NOlap::ISnapshotSchema::TPtr& schema) const;
     };
 
     struct TCommitMeta {
diff --git a/ydb/core/tx/columnshard/engines/index_info.cpp b/ydb/core/tx/columnshard/engines/index_info.cpp
index 56dfa7d92e2..2ca6b57cfaa 100644
--- a/ydb/core/tx/columnshard/engines/index_info.cpp
+++ b/ydb/core/tx/columnshard/engines/index_info.cpp
@@ -7,6 +7,7 @@
 #include <ydb/core/formats/arrow/serializer/batch_only.h>
 #include <ydb/core/formats/arrow/transformer/dictionary.h>
 #include <ydb/core/formats/arrow/serializer/full.h>
+#include <ydb/core/base/appdata.h>
 
 namespace NKikimr::NOlap {
 
@@ -22,11 +23,10 @@ static std::vector<TString> NamesOnly(const std::vector<TNameTypeInfo>& columns)
     return out;
 }
 
-TIndexInfo::TIndexInfo(const TString& name, ui32 id, bool compositeIndexKey)
+TIndexInfo::TIndexInfo(const TString& name, ui32 id)
     : NTable::TScheme::TTableSchema()
     , Id(id)
     , Name(name)
-    , CompositeIndexKey(compositeIndexKey)
 {}
 
 std::shared_ptr<arrow::RecordBatch> TIndexInfo::AddSpecialColumns(const std::shared_ptr<arrow::RecordBatch>& batch, const TSnapshot& snapshot) {
@@ -374,9 +374,16 @@ bool TIndexInfo::DeserializeFromProto(const NKikimrSchemeOp::TColumnTableSchema&
         }
         DefaultCompression = *result;
     }
+
+    CompositeMarks = schema.GetCompositeMarks();
+    CompositeIndexKey = AppData()->FeatureFlags.GetForceColumnTablesCompositeMarks() ? true : CompositeMarks;
     return true;
 }
 
+bool TIndexInfo::CheckAlterScheme(const NKikimrSchemeOp::TColumnTableSchema& scheme) const {
+    return CompositeMarks == scheme.GetCompositeMarks();
+}
+
 std::shared_ptr<arrow::Schema> MakeArrowSchema(const NTable::TScheme::TTableSchema::TColumns& columns, const std::vector<ui32>& ids, bool withSpecials) {
     std::vector<std::shared_ptr<arrow::Field>> fields;
     fields.reserve(withSpecials ? ids.size() + 2 : ids.size());
@@ -412,4 +419,12 @@ std::vector<TNameTypeInfo> GetColumns(const NTable::TScheme::TTableSchema& table
     return out;
 }
 
+std::optional<TIndexInfo> TIndexInfo::BuildFromProto(const NKikimrSchemeOp::TColumnTableSchema& schema) {
+    TIndexInfo result("", 0);
+    if (!result.DeserializeFromProto(schema)) {
+        return std::nullopt;
+    }
+    return result;
+}
+
 } // namespace NKikimr::NOlap
diff --git a/ydb/core/tx/columnshard/engines/index_info.h b/ydb/core/tx/columnshard/engines/index_info.h
index 55c8b90d4d6..9d037c509ad 100644
--- a/ydb/core/tx/columnshard/engines/index_info.h
+++ b/ydb/core/tx/columnshard/engines/index_info.h
@@ -34,7 +34,7 @@ struct TIndexInfo : public NTable::TScheme::TTableSchema {
 private:
     mutable THashMap<ui32, TColumnFeatures> ColumnFeatures;
     mutable THashMap<ui32, std::shared_ptr<arrow::Field>> ArrowColumnByColumnIdCache;
-    TIndexInfo(const TString& name, ui32 id, bool compositeIndexKey = false);
+    TIndexInfo(const TString& name, ui32 id);
     bool DeserializeFromProto(const NKikimrSchemeOp::TColumnTableSchema& schema);
     TColumnFeatures& GetOrCreateColumnFeatures(const ui32 columnId) const;
 public:
@@ -69,21 +69,16 @@ public:
         }
         return true;
     }
+
+    bool CheckAlterScheme(const NKikimrSchemeOp::TColumnTableSchema& scheme) const;
 public:
 
     static TIndexInfo BuildDefault() {
-        TIndexInfo result("dummy", 0, false);
+        TIndexInfo result("dummy", 0);
         return result;
     }
 
-    static std::optional<TIndexInfo> BuildFromProto(const NKikimrSchemeOp::TColumnTableSchema& schema,
-                                                    bool forceCompositeMarks) {
-        TIndexInfo result("", 0, forceCompositeMarks || schema.GetCompositeMarks());
-        if (!result.DeserializeFromProto(schema)) {
-            return {};
-        }
-        return result;
-    }
+    static std::optional<TIndexInfo> BuildFromProto(const NKikimrSchemeOp::TColumnTableSchema& schema);
 
     /// Returns id of the index.
     ui32 GetId() const noexcept {
@@ -182,7 +177,7 @@ public:
 private:
     ui32 Id;
     TString Name;
-    const bool CompositeIndexKey;
+    bool CompositeIndexKey = false;
     mutable std::shared_ptr<arrow::Schema> Schema;
     mutable std::shared_ptr<arrow::Schema> SchemaWithSpecials;
     std::shared_ptr<arrow::Schema> SortingKey;
@@ -192,6 +187,7 @@ private:
     THashSet<TString> RequiredColumns;
     THashSet<ui32> MinMaxIdxColumnsIds;
     std::optional<NArrow::TCompression> DefaultCompression;
+    bool CompositeMarks = false;
 };
 
 std::shared_ptr<arrow::Schema> MakeArrowSchema(const NTable::TScheme::TTableSchema::TColumns& columns, const std::vector<ui32>& ids, bool withSpecials = false);
diff --git a/ydb/core/tx/columnshard/tables_manager.cpp b/ydb/core/tx/columnshard/tables_manager.cpp
index 605c81a1d65..f462ecbfad9 100644
--- a/ydb/core/tx/columnshard/tables_manager.cpp
+++ b/ydb/core/tx/columnshard/tables_manager.cpp
@@ -289,8 +289,7 @@ std::shared_ptr<NOlap::TColumnEngineChanges> TTablesManager::StartIndexCleanup(c
 }
 
 NOlap::TIndexInfo TTablesManager::DeserializeIndexInfoFromProto(const NKikimrSchemeOp::TColumnTableSchema& schema) {
-    bool forceCompositeMarks = AppData()->FeatureFlags.GetForceColumnTablesCompositeMarks();
-    std::optional<NOlap::TIndexInfo> indexInfo = NOlap::TIndexInfo::BuildFromProto(schema, forceCompositeMarks);
+    std::optional<NOlap::TIndexInfo> indexInfo = NOlap::TIndexInfo::BuildFromProto(schema);
     Y_VERIFY(indexInfo);
     return *indexInfo;
 }
diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_create_olap_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_create_olap_table.cpp
index b1b6783e3c3..9c5fd0995c6 100644
--- a/ydb/core/tx/schemeshard/schemeshard__operation_create_olap_table.cpp
+++ b/ydb/core/tx/schemeshard/schemeshard__operation_create_olap_table.cpp
@@ -687,9 +687,6 @@ public:
                 return result;
             }
             tableInfo = tableConstructor.BuildTableInfo(errors);
-            if (tableInfo) {
-                tableInfo->Description.MutableSchema()->SetCompositeMarks(true);
-            }
         }
 
         if (!tableInfo) {
diff --git a/ydb/core/tx/schemeshard/schemeshard_olap_types.cpp b/ydb/core/tx/schemeshard/schemeshard_olap_types.cpp
index 04bea02f57a..edb8513b21e 100644
--- a/ydb/core/tx/schemeshard/schemeshard_olap_types.cpp
+++ b/ydb/core/tx/schemeshard/schemeshard_olap_types.cpp
@@ -317,6 +317,7 @@ namespace NKikimr::NSchemeShard {
         Version = tableSchema.GetVersion();
         Y_VERIFY(tableSchema.HasEngine());
         Engine = tableSchema.GetEngine();
+        CompositeMarksFlag = tableSchema.GetCompositeMarks();
 
         TMap<TString, ui32> keyIndexes;
         ui32 idx = 0;
@@ -348,6 +349,7 @@ namespace NKikimr::NSchemeShard {
     void TOlapSchema::Serialize(NKikimrSchemeOp::TColumnTableSchema& tableSchema) const {
         tableSchema.SetNextColumnId(NextColumnId);
         tableSchema.SetVersion(Version);
+        tableSchema.SetCompositeMarks(CompositeMarksFlag);
 
         Y_VERIFY(HasEngine());
         tableSchema.SetEngine(GetEngineUnsafe());
diff --git a/ydb/core/tx/schemeshard/schemeshard_olap_types.h b/ydb/core/tx/schemeshard/schemeshard_olap_types.h
index f322f2fabe1..1a9a69e6cff 100644
--- a/ydb/core/tx/schemeshard/schemeshard_olap_types.h
+++ b/ydb/core/tx/schemeshard/schemeshard_olap_types.h
@@ -124,6 +124,7 @@ namespace NKikimr::NSchemeShard {
 
         YDB_READONLY(ui32, NextColumnId, 1);
         YDB_READONLY(ui32, Version, 0);
+        YDB_READONLY_FLAG(CompositeMarks, true);
 
     public:
         const TOlapColumnSchema* GetColumnByName(const TString& name) const noexcept {