fix drop column & reset ttl in one TX on CS (#12127)

ydb-platform · Nov 29, 2024 · f58f7ce · f58f7ce
1 parent a4d9d0f
commit f58f7ce
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 16 deletions.
diff --git a/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp b/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp
@@ -10306,6 +10306,43 @@ Y_UNIT_TEST_SUITE(KqpOlapScheme) {
         testTable.SetName(tableName).SetPrimaryKey({ "Key" }).SetSchema(schema).SetColumnFamilies(families);
         testHelper.CreateTable(testTable, EStatus::GENERIC_ERROR);
     }
+
+    Y_UNIT_TEST(DropColumnAndResetTtl) {
+        TKikimrSettings runnerSettings;
+        runnerSettings.WithSampleTables = false;
+        TTestHelper testHelper(runnerSettings);
+
+        TVector<TTestHelper::TColumnSchema> schema = {
+            TTestHelper::TColumnSchema().SetName("id").SetType(NScheme::NTypeIds::Int32).SetNullable(false),
+            TTestHelper::TColumnSchema().SetName("timestamp").SetType(NScheme::NTypeIds::Timestamp).SetNullable(false)
+        };
+
+        TTestHelper::TColumnTable testTable;
+        testTable.SetName("/Root/ColumnTableTest").SetPrimaryKey({"id"}).SetSharding({"id"}).SetSchema(schema);
+        testHelper.CreateTable(testTable);
+
+        {
+            auto alterQuery = TStringBuilder() << R"(
+            --!syntax_v1
+            ALTER OBJECT `)" << testTable.GetName() << R"(` (TYPE TABLE) SET (ACTION=UPSERT_INDEX,
+                NAME=max_pk_int, TYPE=MAX, FEATURES=`{\"column_name\": \"timestamp\"}`))";
+            auto alterResult = testHelper.GetSession().ExecuteSchemeQuery(alterQuery).GetValueSync();
+            UNIT_ASSERT_VALUES_EQUAL_C(alterResult.GetStatus(), EStatus::SUCCESS, alterResult.GetIssues().ToString());
+        }
+
+        {
+            auto alterQuery = TStringBuilder() << "ALTER TABLE `" << testTable.GetName() << "`SET (TTL = Interval(\"PT1H\") ON timestamp);";
+            auto alterResult = testHelper.GetSession().ExecuteSchemeQuery(alterQuery).GetValueSync();
+            UNIT_ASSERT_VALUES_EQUAL_C(alterResult.GetStatus(), EStatus::SUCCESS, alterResult.GetIssues().ToString());
+        }
+
+        {
+            auto alterQuery = TStringBuilder() << "ALTER TABLE `" << testTable.GetName() << "` DROP COLUMN timestamp, RESET (TTL);";
+            auto alterResult = testHelper.GetSession().ExecuteSchemeQuery(alterQuery).GetValueSync();
+            UNIT_ASSERT_VALUES_EQUAL_C(alterResult.GetStatus(), EStatus::SUCCESS, alterResult.GetIssues().ToString());
+        }
+    }
+
 }
 
 Y_UNIT_TEST_SUITE(KqpOlapTypes) {

diff --git a/ydb/core/tx/columnshard/columnshard_ttl.h b/ydb/core/tx/columnshard/columnshard_ttl.h
@@ -48,13 +48,6 @@ class TTtl {
 
     void SetPathTtl(ui64 pathId, TDescription&& descr) {
         if (descr.Eviction) {
-            auto& evict = descr.Eviction;
-            auto it = Columns.find(evict->ColumnName);
-            if (it != Columns.end()) {
-                evict->ColumnName = *it; // replace string dups (memory efficiency)
-            } else {
-                Columns.insert(evict->ColumnName);
-            }
             PathTtls[pathId] = descr;
         } else {
             PathTtls.erase(pathId);
@@ -74,11 +67,16 @@ class TTtl {
         return true;
     }
 
-    const THashSet<TString>& TtlColumns() const { return Columns; }
+    THashSet<TString> TtlColumns() const {
+        THashSet<TString> columns;
+        for (const auto& [pathId, settings] : PathTtls) {
+            columns.insert(settings.Eviction->ColumnName);
+        }
+        return columns;
+    }
 
 private:
     THashMap<ui64, TDescription> PathTtls; // pathId -> ttl
-    THashSet<TString> Columns;
 
     std::shared_ptr<NOlap::TTierInfo> Convert(const TDescription& descr) const
     {

diff --git a/ydb/core/tx/columnshard/tables_manager.cpp b/ydb/core/tx/columnshard/tables_manager.cpp
@@ -317,6 +317,17 @@ void TTablesManager::AddTableVersion(const ui64 pathId, const NOlap::TSnapshot&
     AFL_VERIFY(it != Tables.end());
     auto& table = it->second;
 
+    bool isTtlModified = false;
+    if (versionInfo.HasTtlSettings()) {
+        isTtlModified = true;
+        const auto& ttlSettings = versionInfo.GetTtlSettings();
+        if (ttlSettings.HasEnabled()) {
+            Ttl.SetPathTtl(pathId, TTtl::TDescription(ttlSettings.GetEnabled()));
+        } else {
+            Ttl.DropPathTtl(pathId);
+        }
+    }
+
     if (versionInfo.HasSchemaPresetId()) {
         AFL_VERIFY(!schema);
         Y_ABORT_UNLESS(SchemaPresetsIds.contains(versionInfo.GetSchemaPresetId()));
@@ -330,13 +341,7 @@ void TTablesManager::AddTableVersion(const ui64 pathId, const NOlap::TSnapshot&
         AddSchemaVersion(fakePreset.GetId(), version, *schema, db, manager);
     }
 
-    if (versionInfo.HasTtlSettings()) {
-        const auto& ttlSettings = versionInfo.GetTtlSettings();
-        if (ttlSettings.HasEnabled()) {
-            Ttl.SetPathTtl(pathId, TTtl::TDescription(ttlSettings.GetEnabled()));
-        } else {
-            Ttl.DropPathTtl(pathId);
-        }
+    if (isTtlModified) {
         if (PrimaryIndex && manager->IsReady()) {
             PrimaryIndex->OnTieringModified(manager, Ttl, pathId);
         }