25-1: Preserve column order (#18938)

Author: CyberROFL

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

@@ -3400,11 +3400,11 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
             UNIT_ASSERT_VALUES_EQUAL(decimalType.Scale, scale);
         };
 
-        checkColumn(0,22, 20);
-        checkColumn(3, 1, 0);
-        checkColumn(4, 2, 1);
-        checkColumn(5, 22,9);
-        checkColumn(6, 35, 10);
+        checkColumn(2, 1, 0);
+        checkColumn(3, 2, 1);
+        checkColumn(4, 22, 9);
+        checkColumn(5, 35, 10);
+        checkColumn(6, 22, 20);
     }
 
     Y_UNIT_TEST(AlterTableWithPgColumn) {

ydb/core/tx/schemeshard/schemeshard__conditional_erase.cpp

@@ -292,8 +292,9 @@ struct TSchemeShard::TTxRunConditionalErase: public TSchemeShard::TRwTxBase {
         }
 
         for (const ui32 mainColumnId : mainTable->KeyColumnIds) {
-            Y_ABORT_UNLESS(mainTable->Columns.contains(mainColumnId));
-            const TString& mainKey = mainTable->Columns.at(mainColumnId).Name;
+            auto it = mainTable->Columns.find(mainColumnId);
+            Y_ABORT_UNLESS(it != mainTable->Columns.end());
+            const TString& mainKey = it->second.Name;
 
             if (keys.contains(mainKey)) {
                 continue;
@@ -306,7 +307,7 @@ struct TSchemeShard::TTxRunConditionalErase: public TSchemeShard::TRwTxBase {
         return result;
     }
 
-    static THashMap<TString, ui32> MakeColumnNameToId(const THashMap<ui32, TTableInfo::TColumn>& columns) {
+    static THashMap<TString, ui32> MakeColumnNameToId(const TMap<ui32, TTableInfo::TColumn>& columns) {
         THashMap<TString, ui32> result;
 
         for (const auto& [id, column] : columns) {

ydb/core/tx/schemeshard/schemeshard_info_types.cpp

@@ -596,7 +596,7 @@ TTableInfo::TAlterDataPtr TTableInfo::CreateAlterData(
 
         const auto& ttl = op.GetTTLSettings();
 
-        if (!ValidateTtlSettings(ttl, source ? source->Columns : THashMap<ui32, TColumn>(), alterData->Columns, colName2Id, subDomain, errStr)) {
+        if (!ValidateTtlSettings(ttl, source ? source->Columns : TMap<ui32, TColumn>(), alterData->Columns, colName2Id, subDomain, errStr)) {
             return nullptr;
         }
 

ydb/core/tx/schemeshard/schemeshard_info_types.h

@@ -397,7 +397,7 @@ struct TTableInfo : public TSimpleRefCount<TTableInfo> {
 
         ui32 NextColumnId = 1;
         ui64 AlterVersion = 0;
-        THashMap<ui32, TColumn> Columns;
+        TMap<ui32, TColumn> Columns;
         TVector<ui32> KeyColumnIds;
         bool IsBackup = false;
         bool IsRestore = false;
@@ -441,7 +441,7 @@ struct TTableInfo : public TSimpleRefCount<TTableInfo> {
     ui32 NextColumnId = 1;          // Next unallocated column id
     ui64 AlterVersion = 0;
     ui64 PartitioningVersion = 0;
-    THashMap<ui32, TColumn> Columns;
+    TMap<ui32, TColumn> Columns;
     TVector<ui32> KeyColumnIds;
     bool IsBackup = false;
     bool IsRestore = false;
@@ -3800,8 +3800,8 @@ struct TBackupCollectionInfo : TSimpleRefCount<TBackupCollectionInfo> {
 };
 
 bool ValidateTtlSettings(const NKikimrSchemeOp::TTTLSettings& ttl,
-    const THashMap<ui32, TTableInfo::TColumn>& sourceColumns,
-    const THashMap<ui32, TTableInfo::TColumn>& alterColumns,
+    const TMap<ui32, TTableInfo::TColumn>& sourceColumns,
+    const TMap<ui32, TTableInfo::TColumn>& alterColumns,
     const THashMap<TString, ui32>& colName2Id,
     const TSubDomainInfo& subDomain, TString& errStr);
 

ydb/core/tx/schemeshard/schemeshard_validate_ttl.cpp

@@ -19,8 +19,8 @@ static inline NScheme::TTypeInfo GetType(const TTableInfo::TColumn& col) {
 }
 
 bool ValidateTtlSettings(const NKikimrSchemeOp::TTTLSettings& ttl,
-    const THashMap<ui32, TTableInfo::TColumn>& sourceColumns,
-    const THashMap<ui32, TTableInfo::TColumn>& alterColumns,
+    const TMap<ui32, TTableInfo::TColumn>& sourceColumns,
+    const TMap<ui32, TTableInfo::TColumn>& alterColumns,
     const THashMap<TString, ui32>& colName2Id,
     const TSubDomainInfo& subDomain, TString& errStr)
 {
@@ -39,10 +39,10 @@ bool ValidateTtlSettings(const NKikimrSchemeOp::TTTLSettings& ttl,
 
         const TTableInfo::TColumn* column = nullptr;
         const ui32 colId = it->second;
-        if (alterColumns.contains(colId)) {
-            column = &alterColumns.at(colId);
-        } else if (sourceColumns.contains(colId)) {
-            column = &sourceColumns.at(colId);
+        if (auto x = alterColumns.find(colId); x != alterColumns.end()) {
+            column = &x->second;
+        } else if (auto x = sourceColumns.find(colId); x != sourceColumns.end()) {
+            column = &x->second;
         } else {
             Y_ABORT_UNLESS("Unknown column");
         }

ydb/core/tx/schemeshard/ut_base/ut_base.cpp

@@ -7,6 +7,7 @@
 
 #include <util/generic/size_literals.h>
 #include <util/string/cast.h>
+#include <util/string/printf.h>
 
 #include <locale>
 
@@ -11784,4 +11785,32 @@ Y_UNIT_TEST_SUITE(TSchemeShardTest) {
             });
         }
     }
+
+    Y_UNIT_TEST(PreserveColumnOrder) {
+        TTestBasicRuntime runtime;
+        TTestEnv env(runtime);
+
+        static constexpr ui32 columns = 29;
+
+        TStringBuilder schema;
+        for (ui32 i = 1; i <= columns; ++i) {
+            schema << "Columns { Name: \"" << Sprintf("col%0*" PRIu32, 2, i) << "\" Type: \"Utf8\" }" << Endl;
+        }
+        schema << "Name: \"Table\"" << Endl;
+        schema << "KeyColumnNames: [\"col01\"]" << Endl;
+
+        ui64 txId = 100;
+        TestCreateTable(runtime, ++txId, "/MyRoot", schema);
+        env.TestWaitNotification(runtime, txId);
+
+        TestDescribeResult(DescribePath(runtime, "/MyRoot/Table"), {
+            NLs::PathExist, [&] (const NKikimrScheme::TEvDescribeSchemeResult& record) {
+                const auto& table = record.GetPathDescription().GetTable();
+                UNIT_ASSERT_VALUES_EQUAL(table.ColumnsSize(), columns);
+                for (ui32 i = 1; i <= columns; ++i) {
+                    UNIT_ASSERT_VALUES_EQUAL(table.GetColumns(i - 1).GetName(), Sprintf("col%0*" PRIu32, 2, i));
+                }
+            }
+        });
+    }
 }