Alter index is not supported for vector index (#7005)

azevaykin · web-flow · commit f36b8b559d5a · 2024-07-25T12:15:18.000+03:00
diff --git a/ydb/core/base/table_index.cpp b/ydb/core/base/table_index.cpp
@@ -143,6 +143,14 @@ bool IsCompatibleIndex(NKikimrSchemeOp::EIndexType indexType, const TTableColumn
     return true;
 }
 
+TVector<TString> GetImplTables(NKikimrSchemeOp::EIndexType indexType) {
+    if (indexType == NKikimrSchemeOp::EIndexType::EIndexTypeGlobalVectorKmeansTree) {
+        return { NTableVectorKmeansTreeIndex::LevelTable, NTableVectorKmeansTreeIndex::PostingTable };
+    } else {
+        return { ImplTable };
+    }
+}
+
 bool IsImplTable(std::string_view tableName) {
     return Contains(ImplTables, tableName);
 }
diff --git a/ydb/core/base/table_index.h b/ydb/core/base/table_index.h
@@ -23,6 +23,8 @@ inline constexpr const char* ImplTable = "indexImplTable";
 
 bool IsCompatibleIndex(NKikimrSchemeOp::EIndexType type, const TTableColumns& table, const TIndexColumns& index, TString& explain);
 TTableColumns CalcTableImplDescription(NKikimrSchemeOp::EIndexType type, const TTableColumns& table, const TIndexColumns& index);
+
+TVector<TString> GetImplTables(NKikimrSchemeOp::EIndexType indexType);
 bool IsImplTable(std::string_view tableName);
 bool IsTmpImplTable(std::string_view tableName);
 
diff --git a/ydb/core/grpc_services/rpc_describe_table.cpp b/ydb/core/grpc_services/rpc_describe_table.cpp
@@ -28,8 +28,9 @@ class TDescribeTableRPC : public TRpcSchemeRequestActor<TDescribeTableRPC, TEvDe
         if (AppData()->AllowPrivateTableDescribeForTest) {
            return true;
         }
-        
-        if (path.EndsWith(TStringBuilder() << "/" << NTableIndex::ImplTable)) {
+
+        auto pathElements = ::NKikimr::SplitPath(path);
+        if (pathElements.size() != 0 && NTableIndex::IsImplTable(pathElements.back())) {
             return true;
         }
 
diff --git a/ydb/core/kqp/gateway/kqp_metadata_loader.cpp b/ydb/core/kqp/gateway/kqp_metadata_loader.cpp
@@ -616,38 +616,39 @@ NThreading::TFuture<TTableMetadataResult> TKqpTableMetadataLoader::LoadIndexMeta
 
     for (size_t i = 0; i < indexesCount; i++) {
         const auto& index = tableMetadata->Indexes[i];
-        auto indexTablePath = NSchemeHelpers::CreateIndexTablePath(tableName, index.Name);
-
-        if (!index.SchemaVersion) {
-            LOG_DEBUG_S(*ActorSystem, NKikimrServices::KQP_GATEWAY, "Load index metadata without schema version check index: " << index.Name);
-            children.push_back(
-                LoadTableMetadata(cluster, indexTablePath,
-                    TLoadTableMetadataSettings().WithPrivateTables(true), database, userToken)
-                    .Apply([i, tableMetadata](const TFuture<TTableMetadataResult>& result) {
-                        auto value = result.GetValue();
-                        UpdateMetadataIfSuccess(tableMetadata, i, value);
-                        return static_cast<TGenericResult>(value);
-                    })
-            );
+        const auto indexTablePaths = NSchemeHelpers::CreateIndexTablePath(tableName, index.Type, index.Name);
+        for (const auto& indexTablePath : indexTablePaths) {
+            if (!index.SchemaVersion) {
+                LOG_DEBUG_S(*ActorSystem, NKikimrServices::KQP_GATEWAY, "Load index metadata without schema version check index: " << index.Name);
+                children.push_back(
+                    LoadTableMetadata(cluster, indexTablePath,
+                        TLoadTableMetadataSettings().WithPrivateTables(true), database, userToken)
+                        .Apply([i, tableMetadata](const TFuture<TTableMetadataResult>& result) {
+                            auto value = result.GetValue();
+                            UpdateMetadataIfSuccess(tableMetadata, i, value);
+                            return static_cast<TGenericResult>(value);
+                        })
+                );
 
-        } else {
-            LOG_DEBUG_S(*ActorSystem, NKikimrServices::KQP_GATEWAY, "Load index metadata with schema version check"
-                << "index: " << index.Name
-                << "pathId: " << index.LocalPathId
-                << "ownerId: " << index.PathOwnerId
-                << "schemaVersion: " << index.SchemaVersion
-                << "tableOwnerId: " << tableOwnerId);
-             auto ownerId = index.PathOwnerId ? index.PathOwnerId : tableOwnerId; //for compat with 20-2
-             children.push_back(
-                 LoadIndexMetadataByPathId(cluster,
-                     NKikimr::TIndexId(ownerId, index.LocalPathId, index.SchemaVersion), indexTablePath, database, userToken)
-                     .Apply([i, tableMetadata](const TFuture<TTableMetadataResult>& result) {
-                         auto value = result.GetValue();
-                         UpdateMetadataIfSuccess(tableMetadata, i, value);
-                         return static_cast<TGenericResult>(value);
-                     })
-             );
+            } else {
+                LOG_DEBUG_S(*ActorSystem, NKikimrServices::KQP_GATEWAY, "Load index metadata with schema version check"
+                    << "index: " << index.Name
+                    << "pathId: " << index.LocalPathId
+                    << "ownerId: " << index.PathOwnerId
+                    << "schemaVersion: " << index.SchemaVersion
+                    << "tableOwnerId: " << tableOwnerId);
+                auto ownerId = index.PathOwnerId ? index.PathOwnerId : tableOwnerId; //for compat with 20-2
+                children.push_back(
+                    LoadIndexMetadataByPathId(cluster,
+                        NKikimr::TIndexId(ownerId, index.LocalPathId, index.SchemaVersion), indexTablePath, database, userToken)
+                        .Apply([i, tableMetadata](const TFuture<TTableMetadataResult>& result) {
+                            auto value = result.GetValue();
+                            UpdateMetadataIfSuccess(tableMetadata, i, value);
+                            return static_cast<TGenericResult>(value);
+                        })
+                );
 
+            }
         }
     }
 
diff --git a/ydb/core/kqp/gateway/utils/scheme_helpers.cpp b/ydb/core/kqp/gateway/utils/scheme_helpers.cpp
@@ -46,8 +46,14 @@ bool SplitTablePath(const TString& tableName, const TString& database, std::pair
     }
 }
 
-TString CreateIndexTablePath(const TString& tableName, const TString& indexName) {
-    return tableName + "/" + indexName + "/" + NTableIndex::ImplTable;
+TVector<TString> CreateIndexTablePath(const TString& tableName, NYql::TIndexDescription::EType indexType, const TString& indexName) {
+    auto implTables = NTableIndex::GetImplTables(NYql::TIndexDescription::ConvertIndexType(indexType));
+    TVector<TString> paths;
+    paths.reserve(implTables.size());
+    for (const auto& implTable : implTables) {
+        paths.emplace_back(TStringBuilder() << tableName << "/" << indexName << "/" << implTable);
+    }
+    return paths;
 }
 
 bool SetDatabaseForLoginOperation(TString& result, bool getDomainLoginOnly, TMaybe<TString> domainName,
diff --git a/ydb/core/kqp/gateway/utils/scheme_helpers.h b/ydb/core/kqp/gateway/utils/scheme_helpers.h
@@ -22,7 +22,7 @@ bool TrySplitTablePath(const TString& path, std::pair<TString, TString>& result,
 bool SplitTablePath(const TString& tableName, const TString& database, std::pair<TString, TString>& pathPair,
     TString& error, bool createDir);
 
-TString CreateIndexTablePath(const TString& tableName, const TString& indexName);
+TVector<TString> CreateIndexTablePath(const TString& tableName, NYql::TIndexDescription::EType indexType, const TString& indexName);
 
 bool SetDatabaseForLoginOperation(TString& result, bool getDomainLoginOnly, TMaybe<TString> domainName,
     const TString& database);
diff --git a/ydb/core/kqp/provider/yql_kikimr_exec.cpp b/ydb/core/kqp/provider/yql_kikimr_exec.cpp
@@ -1682,9 +1682,25 @@ class TKiSinkCallableExecutionTransformer : public TAsyncCallbackTransformer<TKi
                     for (const auto& indexSetting : listNode) {
                         auto settingName = indexSetting.Name().Value();
                         if (settingName == "indexName") {
-                            auto indexName = indexSetting.Value().Cast<TCoAtom>().StringValue();
-                            auto indexTablePath = NKikimr::NKqp::NSchemeHelpers::CreateIndexTablePath(table.Metadata->Name, indexName);
-                            alterTableRequest.set_path(std::move(indexTablePath));
+                            const auto indexName = indexSetting.Value().Cast<TCoAtom>().StringValue();
+                            const auto indexIter = std::find_if(table.Metadata->Indexes.begin(), table.Metadata->Indexes.end(), [&indexName] (const auto& index) {
+                                return index.Name == indexName;
+                            });
+                            if (indexIter == table.Metadata->Indexes.end()) {
+                                ctx.AddError(
+                                    YqlIssue(ctx.GetPosition(indexSetting.Name().Pos()),
+                                        TIssuesIds::KIKIMR_SCHEME_ERROR,
+                                        TStringBuilder() << "Unknown index name: " << indexName));                                
+                                return SyncError();
+                            }                            
+                            auto indexTablePaths = NKikimr::NKqp::NSchemeHelpers::CreateIndexTablePath(table.Metadata->Name, indexIter->Type, indexName);
+                            if (indexTablePaths.size() != 1) {
+                                ctx.AddError(
+                                    TIssue(ctx.GetPosition(indexSetting.Name().Pos()),
+                                        TStringBuilder() << "Only index with one impl table is supported"));
+                                return SyncError();
+                            }
+                            alterTableRequest.set_path(std::move(indexTablePaths[0]));
                         } else if (settingName == "tableSettings") {
                             auto tableSettings = indexSetting.Value().Cast<TCoNameValueTupleList>();
                             for (const auto& tableSetting : tableSettings) {
diff --git a/ydb/core/kqp/provider/yql_kikimr_opt_build.cpp b/ydb/core/kqp/provider/yql_kikimr_opt_build.cpp
@@ -130,13 +130,16 @@ struct TKiExploreTxResults {
         auto view = key.GetView();
         if (view && view->Name) {
             const auto& indexName = view->Name;
-            const auto indexTablePath = NKikimr::NKqp::NSchemeHelpers::CreateIndexTablePath(tableMeta->Name, indexName);
 
             auto indexIt = std::find_if(tableMeta->Indexes.begin(), tableMeta->Indexes.end(), [&indexName](const auto& index){
                 return index.Name == indexName;
             });
             YQL_ENSURE(indexIt != tableMeta->Indexes.end(), "Index not found");
 
+            const auto indexTablePaths = NKikimr::NKqp::NSchemeHelpers::CreateIndexTablePath(tableMeta->Name, indexIt->Type, indexName);
+            YQL_ENSURE(indexTablePaths.size() == 1, "Only index with one impl table is supported");
+            const auto indexTablePath = indexTablePaths[0];
+
             THashSet<TString> indexColumns;
             indexColumns.reserve(indexIt->KeyColumns.size() + indexIt->DataColumns.size());
             for (const auto& keyColumn : indexIt->KeyColumns) {
@@ -180,7 +183,9 @@ struct TKiExploreTxResults {
                 continue;
             }
 
-            const auto indexTable = NKikimr::NKqp::NSchemeHelpers::CreateIndexTablePath(tableMeta->Name, index.Name);
+            const auto indexTables = NKikimr::NKqp::NSchemeHelpers::CreateIndexTablePath(tableMeta->Name, index.Type, index.Name);
+            YQL_ENSURE(indexTables.size() == 1, "Only index with one impl table is supported");
+            const auto indexTable = indexTables[0];
 
             ops[tableMeta->Name] |= TPrimitiveYdbOperation::Read;
             ops[indexTable] = TPrimitiveYdbOperation::Write;
@@ -202,7 +207,10 @@ struct TKiExploreTxResults {
                 continue;
             }
 
-            const auto indexTable = NKikimr::NKqp::NSchemeHelpers::CreateIndexTablePath(tableMeta->Name, index.Name);
+            const auto indexTables = NKikimr::NKqp::NSchemeHelpers::CreateIndexTablePath(tableMeta->Name, index.Type, index.Name);
+            YQL_ENSURE(indexTables.size() == 1, "Only index with one impl table is supported");
+            const auto indexTable = indexTables[0];
+
             for (const auto& column : index.KeyColumns) {
                 if (updateColumns.contains(column)) {
                     // delete old index values and upsert rows into index table
diff --git a/ydb/core/kqp/ut/common/kqp_ut_common.cpp b/ydb/core/kqp/ut/common/kqp_ut_common.cpp
@@ -502,6 +502,7 @@ void TKikimrRunner::Initialize(const TKikimrSettings& settings) {
     SetupLogLevelFromTestParam(NKikimrServices::KQP_TASKS_RUNNER);
     SetupLogLevelFromTestParam(NKikimrServices::KQP_EXECUTER);
     SetupLogLevelFromTestParam(NKikimrServices::TX_PROXY_SCHEME_CACHE);
+    SetupLogLevelFromTestParam(NKikimrServices::TX_PROXY);
     SetupLogLevelFromTestParam(NKikimrServices::SCHEME_BOARD_REPLICA);
     SetupLogLevelFromTestParam(NKikimrServices::KQP_WORKER);
     SetupLogLevelFromTestParam(NKikimrServices::KQP_SESSION);
diff --git a/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp b/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp
@@ -2472,6 +2472,59 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
         }        
     }
 
+    Y_UNIT_TEST(AlterTableAlterVectorIndex) {
+        NKikimrConfig::TFeatureFlags featureFlags;
+        featureFlags.SetEnableVectorIndex(true);
+        auto settings = TKikimrSettings().SetFeatureFlags(featureFlags);
+        TKikimrRunner kikimr(settings);
+        auto db = kikimr.GetTableClient();
+        auto session = db.CreateSession().GetValueSync().GetSession();
+        {
+            TString create_index_query = R"(
+                --!syntax_v1
+                CREATE TABLE `/Root/TestTable` (
+                    Key Uint64,
+                    Embedding String,
+                    PRIMARY KEY (Key),
+                    INDEX vector_idx 
+                        GLOBAL USING vector_kmeans_tree 
+                        ON (Embedding)
+                        WITH (similarity=cosine, vector_type=bit, vector_dimension=1)
+                );
+            )";
+            auto result = session.ExecuteSchemeQuery(create_index_query).ExtractValueSync();
+            UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
+        }
+        {
+            auto describe = session.DescribeTable("/Root/TestTable/vector_idx/indexImplPostingTable").GetValueSync();
+            UNIT_ASSERT_C(describe.IsSuccess(), describe.GetIssues().ToString());
+            auto indexDesc = describe.GetTableDescription();
+            constexpr int defaultPartitionSizeMb = 2048;
+            UNIT_ASSERT_VALUES_EQUAL(indexDesc.GetPartitioningSettings().GetPartitionSizeMb(), defaultPartitionSizeMb);
+        }        
+        {
+            auto result = session.ExecuteSchemeQuery(R"(
+                        ALTER TABLE `/Root/TestTable` ALTER INDEX vector_idx SET AUTO_PARTITIONING_MIN_PARTITIONS_COUNT 1;
+                    )").ExtractValueSync();
+            UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::GENERIC_ERROR, result.GetIssues().ToString());
+            UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToString(), "Only index with one impl table is supported" );
+        }
+    }    
+
+    Y_UNIT_TEST(AlterTableAlterMissedIndex) {
+        TKikimrRunner kikimr;
+        auto db = kikimr.GetTableClient();
+        auto session = db.CreateSession().GetValueSync().GetSession();
+        CreateSampleTablesWithIndex(session);      
+        {
+            auto result = session.ExecuteSchemeQuery(R"(
+                        ALTER TABLE `/Root/SecondaryKeys` ALTER INDEX WrongIndexName SET AUTO_PARTITIONING_MIN_PARTITIONS_COUNT 1;
+                    )").ExtractValueSync();
+            UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, result.GetIssues().ToString());
+            UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToString(), "Unknown index name: WrongIndexName");
+        }
+    }    
+
     Y_UNIT_TEST(AlterIndexImplTable) {
         TKikimrRunner kikimr;
         auto db = kikimr.GetTableClient();
@@ -2695,6 +2748,7 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
                         WITH (similarity=inner_product, vector_type=float, vector_dimension=1024)
                 );
             )";
+            
             auto result = session.ExecuteSchemeQuery(create_index_query).ExtractValueSync();
 
             UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
@@ -2714,7 +2768,14 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
             UNIT_ASSERT_VALUES_EQUAL(indexDesc.GetVectorIndexSettings()->VectorType, NYdb::NTable::TVectorIndexSettings::EVectorType::Float);
             UNIT_ASSERT_VALUES_EQUAL(indexDesc.GetVectorIndexSettings()->VectorDimension, 1024);
         }
-    } 
+        {
+            auto describeLevelTable = session.DescribeTable("/Root/TestTable/vector_idx/indexImplLevelTable").GetValueSync();
+            UNIT_ASSERT_C(describeLevelTable.IsSuccess(), describeLevelTable.GetIssues().ToString());
+            auto describePostingTable = session.DescribeTable("/Root/TestTable/vector_idx/indexImplPostingTable").GetValueSync();
+            UNIT_ASSERT_C(describePostingTable.IsSuccess(), describePostingTable.GetIssues().ToString());
+        }   
+    }
+
 
     Y_UNIT_TEST(CreateTableWithVectorIndexCovered) {
         NKikimrConfig::TFeatureFlags featureFlags;

Original file line number	Diff line number	Diff line change
`@@ -143,6 +143,14 @@ bool IsCompatibleIndex(NKikimrSchemeOp::EIndexType indexType, const TTableColumn`
`143`	`143`	`return true;`
`144`	`144`	`}`
`145`	`145`
	`146`	`+TVector<TString> GetImplTables(NKikimrSchemeOp::EIndexType indexType) {`
	`147`	`+ if (indexType == NKikimrSchemeOp::EIndexType::EIndexTypeGlobalVectorKmeansTree) {`
	`148`	`+ return { NTableVectorKmeansTreeIndex::LevelTable, NTableVectorKmeansTreeIndex::PostingTable };`
	`149`	`+ } else {`
	`150`	`+ return { ImplTable };`
	`151`	`+ }`
	`152`	`+}`
	`153`	`+`
`146`	`154`	`bool IsImplTable(std::string_view tableName) {`
`147`	`155`	`return Contains(ImplTables, tableName);`
`148`	`156`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,8 +28,9 @@ class TDescribeTableRPC : public TRpcSchemeRequestActor<TDescribeTableRPC, TEvDe`
`28`	`28`	`if (AppData()->AllowPrivateTableDescribeForTest) {`
`29`	`29`	`return true;`
`30`	`30`	`}`
`31`		`-`
`32`		`- if (path.EndsWith(TStringBuilder() << "/" << NTableIndex::ImplTable)) {`
	`31`	`+`
	`32`	`+ auto pathElements = ::NKikimr::SplitPath(path);`
	`33`	`+ if (pathElements.size() != 0 && NTableIndex::IsImplTable(pathElements.back())) {`
`33`	`34`	`return true;`
`34`	`35`	`}`
`35`	`36`
Original file line number	Diff line number	Diff line change
`@@ -46,8 +46,14 @@ bool SplitTablePath(const TString& tableName, const TString& database, std::pair`
`46`	`46`	`}`
`47`	`47`	`}`
`48`	`48`
`49`		`-TString CreateIndexTablePath(const TString& tableName, const TString& indexName) {`
`50`		`- return tableName + "/" + indexName + "/" + NTableIndex::ImplTable;`
	`49`	`+TVector<TString> CreateIndexTablePath(const TString& tableName, NYql::TIndexDescription::EType indexType, const TString& indexName) {`
	`50`	`+ auto implTables = NTableIndex::GetImplTables(NYql::TIndexDescription::ConvertIndexType(indexType));`
	`51`	`+ TVector<TString> paths;`
	`52`	`+ paths.reserve(implTables.size());`
	`53`	`+ for (const auto& implTable : implTables) {`
	`54`	`+ paths.emplace_back(TStringBuilder() << tableName << "/" << indexName << "/" << implTable);`
	`55`	`+ }`
	`56`	`+ return paths;`
`51`	`57`	`}`
`52`	`58`
`53`	`59`	`bool SetDatabaseForLoginOperation(TString& result, bool getDomainLoginOnly, TMaybe<TString> domainName,`