review fix #1

- Change signature of AddSecondaryIndex function to receive TIndexDescription. It should be much more future proof. Besides TAlterTableSettings already supports adding indexes using TIndexDescription. This change will unify TTableBuilder and TAlterTableSettings in this regard.

Author: jepett0

ydb/public/sdk/cpp/client/ydb_table/table.cpp

@@ -469,14 +469,8 @@ class TTableDescription::TImpl {
         Indexes_.emplace_back(TIndexDescription(indexName, type, indexColumns, dataColumns));
     }
 
-    void AddSecondaryIndex(
-        const TString& indexName,
-        EIndexType type,
-        const TVector<TString>& indexColumns,
-        const TVector<TString>& dataColumns,
-        const TGlobalIndexSettings& globalIndexSettings
-    ) {
-        Indexes_.emplace_back(TIndexDescription(indexName, type, indexColumns, dataColumns, { globalIndexSettings }));
+    void AddSecondaryIndex(const TIndexDescription& indexDescription) {
+        Indexes_.emplace_back(indexDescription);
     }
 
     void AddVectorIndex(const TString& indexName, EIndexType type, const TVector<TString>& indexColumns, const TVectorIndexSettings& vectorIndexSettings) {
@@ -759,14 +753,8 @@ void TTableDescription::AddSecondaryIndex(const TString& indexName, EIndexType t
     Impl_->AddSecondaryIndex(indexName, type, indexColumns, dataColumns);
 }
 
-void TTableDescription::AddSecondaryIndex(
-    const TString& indexName,
-    EIndexType type,
-    const TVector<TString>& indexColumns,
-    const TVector<TString>& dataColumns,
-    const TGlobalIndexSettings& globalIndexSettings
-) {
-    Impl_->AddSecondaryIndex(indexName, type, indexColumns, dataColumns, globalIndexSettings);
+void TTableDescription::AddSecondaryIndex(const TIndexDescription& indexDescription) {
+    Impl_->AddSecondaryIndex(indexDescription);
 }
 
 void TTableDescription::AddSyncSecondaryIndex(const TString& indexName, const TVector<TString>& indexColumns) {
@@ -1193,14 +1181,8 @@ TTableBuilder& TTableBuilder::SetPrimaryKeyColumn(const TString& primaryKeyColum
     return *this;
 }
 
-TTableBuilder& TTableBuilder::AddSecondaryIndex(
-    const TString& indexName,
-    EIndexType type,
-    const TVector<TString>& indexColumns,
-    const TVector<TString>& dataColumns,
-    const TGlobalIndexSettings& globalIndexSettings
-) {
-    TableDescription_.AddSecondaryIndex(indexName, type, indexColumns, dataColumns, globalIndexSettings);
+TTableBuilder& TTableBuilder::AddSecondaryIndex(const TIndexDescription& indexDescription) {
+    TableDescription_.AddSecondaryIndex(indexDescription);
     return *this;
 }
 

ydb/public/sdk/cpp/client/ydb_table/table.h

@@ -642,13 +642,7 @@ class TTableDescription {
     // common
     void AddSecondaryIndex(const TString& indexName, EIndexType type, const TVector<TString>& indexColumns);
     void AddSecondaryIndex(const TString& indexName, EIndexType type, const TVector<TString>& indexColumns, const TVector<TString>& dataColumns);
-    void AddSecondaryIndex(
-        const TString& indexName,
-        EIndexType type,
-        const TVector<TString>& indexColumns,
-        const TVector<TString>& dataColumns,
-        const TGlobalIndexSettings& globalIndexSettings
-    );
+    void AddSecondaryIndex(const TIndexDescription& indexDescription);
     // sync
     void AddSyncSecondaryIndex(const TString& indexName, const TVector<TString>& indexColumns);
     void AddSyncSecondaryIndex(const TString& indexName, const TVector<TString>& indexColumns, const TVector<TString>& dataColumns);
@@ -862,13 +856,7 @@ class TTableBuilder {
     TTableBuilder& SetPrimaryKeyColumn(const TString& primaryKeyColumn);
 
     // common
-    TTableBuilder& AddSecondaryIndex(
-        const TString& indexName,
-        EIndexType type,
-        const TVector<TString>& indexColumns,
-        const TVector<TString>& dataColumns,
-        const TGlobalIndexSettings& globalIndexSettings
-    );
+    TTableBuilder& AddSecondaryIndex(const TIndexDescription& indexDescription);
     TTableBuilder& AddSecondaryIndex(const TString& indexName, EIndexType type, const TVector<TString>& indexColumns, const TVector<TString>& dataColumns);
     TTableBuilder& AddSecondaryIndex(const TString& indexName, EIndexType type, const TVector<TString>& indexColumns);
     TTableBuilder& AddSecondaryIndex(const TString& indexName, EIndexType type, const TString& indexColumn);

ydb/services/ydb/backup_ut/ydb_backup_ut.cpp

@@ -279,7 +279,7 @@ void TestIndexTableSplitBoundariesArePreserved(
             .AddNullableColumn("Key", EPrimitiveType::Uint32)
             .AddNullableColumn("Value", EPrimitiveType::Uint32)
             .SetPrimaryKeyColumn("Key")
-            .AddSecondaryIndex("byValue", EIndexType::GlobalSync, { "Value" }, {}, indexSettings);
+            .AddSecondaryIndex(TIndexDescription("byValue", EIndexType::GlobalSync, { "Value" }, {}, { indexSettings }));
 
         const auto result = session.CreateTable(table, tableBuilder.Build()).ExtractValueSync();
         UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString());