From d1b40b7742706a95b41d4067a3581c46c7797ead Mon Sep 17 00:00:00 2001 From: XuanYang-cn Date: Thu, 18 Sep 2025 14:37:07 +0800 Subject: [PATCH] fix: Rename Collection use wrong method to check if collection exists (#44436) See also: #44435 Signed-off-by: yangxuan --- internal/rootcoord/rename_collection_task.go | 12 +- .../rootcoord/rename_collection_task_test.go | 115 ++++++++++++------ 2 files changed, 88 insertions(+), 39 deletions(-) diff --git a/internal/rootcoord/rename_collection_task.go b/internal/rootcoord/rename_collection_task.go index 4d92522351..71eff9884f 100644 --- a/internal/rootcoord/rename_collection_task.go +++ b/internal/rootcoord/rename_collection_task.go @@ -25,6 +25,7 @@ import ( "github.com/milvus-io/milvus/internal/util/hookutil" "github.com/milvus-io/milvus/internal/util/proxyutil" "github.com/milvus-io/milvus/pkg/v2/util" + "github.com/milvus-io/milvus/pkg/v2/util/typeutil" ) type renameCollectionTask struct { @@ -63,13 +64,13 @@ func (t *renameCollectionTask) Execute(ctx context.Context) error { return fmt.Errorf("unsupported use an alias to rename collection, alias:%s", t.Req.GetOldName()) } - collID := t.core.meta.GetCollectionID(ctx, t.Req.GetDbName(), t.Req.GetOldName()) - if collID == 0 { + collInfo, err := t.core.meta.GetCollectionByName(ctx, t.Req.GetDbName(), t.Req.GetOldName(), typeutil.MaxTimestamp) + if err != nil { return fmt.Errorf("collection not found in database, collection: %s, database: %s", t.Req.GetOldName(), t.Req.GetDbName()) } // check old collection doesn't have aliases if renaming databases - aliases := t.core.meta.ListAliasesByID(ctx, collID) + aliases := t.core.meta.ListAliasesByID(ctx, collInfo.CollectionID) if len(aliases) > 0 && targetDB != t.Req.GetDbName() { return fmt.Errorf("fail to rename collection to different database, must drop all aliases of collection %s before rename", t.Req.GetOldName()) } @@ -79,7 +80,8 @@ func (t *renameCollectionTask) Execute(ctx context.Context) error { return fmt.Errorf("cannot rename collection to an existing alias: %s", t.Req.GetNewName()) } - if existingCollID := t.core.meta.GetCollectionID(ctx, targetDB, t.Req.GetNewName()); existingCollID != 0 { + _, err = t.core.meta.GetCollectionByName(ctx, targetDB, t.Req.GetNewName(), typeutil.MaxTimestamp) + if err == nil { return fmt.Errorf("duplicated new collection name %s in database %s with other collection name or alias", t.Req.GetNewName(), targetDB) } @@ -101,7 +103,7 @@ func (t *renameCollectionTask) Execute(ctx context.Context) error { baseStep: baseStep{core: t.core}, dbName: t.Req.GetDbName(), collectionNames: append(aliases, t.Req.GetOldName()), - collectionID: collID, + collectionID: collInfo.CollectionID, ts: ts, opts: []proxyutil.ExpireCacheOpt{proxyutil.SetMsgType(commonpb.MsgType_RenameCollection)}, }) diff --git a/internal/rootcoord/rename_collection_task_test.go b/internal/rootcoord/rename_collection_task_test.go index 96c5d03aa6..71787c7c4e 100644 --- a/internal/rootcoord/rename_collection_task_test.go +++ b/internal/rootcoord/rename_collection_task_test.go @@ -134,11 +134,15 @@ func Test_renameCollectionTask_Prepare(t *testing.T) { "db1", "old_collection", ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, "db1", "old_collection", - ).Return(collectionID) + mock.AnythingOfType("uint64"), + ).Return(&model.Collection{ + CollectionID: collectionID, + Name: "old_collection", + }, nil) meta.On("ListAliasesByID", mock.Anything, collectionID, @@ -148,11 +152,12 @@ func Test_renameCollectionTask_Prepare(t *testing.T) { "db2", "new_collection", ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, "db2", "new_collection", - ).Return(int64(0)) + mock.AnythingOfType("uint64"), + ).Return(nil, errors.New("not found")) meta.On("RenameCollection", mock.Anything, "db1", @@ -195,11 +200,12 @@ func Test_renameCollectionTask_Execute(t *testing.T) { mock.Anything, oldName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, mock.Anything, oldName, - ).Return(int64(0)) + mock.AnythingOfType("uint64"), + ).Return(nil, errors.New("collection not found")) core := newTestCore(withMeta(meta)) task := &renameCollectionTask{ @@ -230,11 +236,15 @@ func Test_renameCollectionTask_Execute(t *testing.T) { mock.Anything, oldName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, mock.Anything, oldName, - ).Return(collectionID) + mock.AnythingOfType("uint64"), + ).Return(&model.Collection{ + CollectionID: collectionID, + Name: oldName, + }, nil) meta.On("ListAliasesByID", mock.Anything, collectionID, @@ -244,11 +254,12 @@ func Test_renameCollectionTask_Execute(t *testing.T) { mock.Anything, newName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, mock.Anything, newName, - ).Return(int64(0)) + mock.AnythingOfType("uint64"), + ).Return(nil, errors.New("not found")) meta.On("RenameCollection", mock.Anything, mock.Anything, @@ -287,11 +298,15 @@ func Test_renameCollectionTask_Execute(t *testing.T) { mock.Anything, oldName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, mock.Anything, oldName, - ).Return(collectionID) + mock.AnythingOfType("uint64"), + ).Return(&model.Collection{ + CollectionID: collectionID, + Name: oldName, + }, nil) meta.On("ListAliasesByID", mock.Anything, collectionID, @@ -301,11 +316,12 @@ func Test_renameCollectionTask_Execute(t *testing.T) { mock.Anything, newName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, mock.Anything, newName, - ).Return(int64(0)) + mock.AnythingOfType("uint64"), + ).Return(nil, errors.New("not found")) meta.On("RenameCollection", mock.Anything, mock.Anything, @@ -345,21 +361,26 @@ func Test_renameCollectionTask_Execute(t *testing.T) { mock.Anything, oldName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, mock.Anything, oldName, - ).Return(collectionID) + mock.AnythingOfType("uint64"), + ).Return(&model.Collection{ + CollectionID: collectionID, + Name: oldName, + }, nil) meta.On("IsAlias", mock.Anything, mock.Anything, newName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, mock.Anything, newName, - ).Return(int64(0)) + mock.AnythingOfType("uint64"), + ).Return(nil, errors.New("not found")) meta.On("ListAliasesByID", mock.Anything, mock.Anything, @@ -425,11 +446,15 @@ func Test_renameCollectionTask_Execute(t *testing.T) { "db1", oldName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, "db1", oldName, - ).Return(collectionID) + mock.AnythingOfType("uint64"), + ).Return(&model.Collection{ + CollectionID: collectionID, + Name: oldName, + }, nil) meta.On("ListAliasesByID", mock.Anything, collectionID, @@ -497,11 +522,15 @@ func Test_renameCollectionTask_Execute(t *testing.T) { mock.Anything, oldName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, mock.Anything, oldName, - ).Return(collectionID) + mock.AnythingOfType("uint64"), + ).Return(&model.Collection{ + CollectionID: collectionID, + Name: oldName, + }, nil) meta.On("ListAliasesByID", mock.Anything, collectionID, @@ -542,11 +571,15 @@ func Test_renameCollectionTask_Execute(t *testing.T) { mock.Anything, oldName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, mock.Anything, oldName, - ).Return(collectionID) + mock.AnythingOfType("uint64"), + ).Return(&model.Collection{ + CollectionID: collectionID, + Name: oldName, + }, nil) meta.On("ListAliasesByID", mock.Anything, collectionID, @@ -556,11 +589,15 @@ func Test_renameCollectionTask_Execute(t *testing.T) { mock.Anything, newName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, mock.Anything, newName, - ).Return(existingCollID) + mock.AnythingOfType("uint64"), + ).Return(&model.Collection{ + CollectionID: existingCollID, + Name: newName, + }, nil) core := newTestCore(withMeta(meta)) task := &renameCollectionTask{ @@ -612,21 +649,26 @@ func Test_renameCollectionTask_Execute(t *testing.T) { oldDB, oldName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, oldDB, oldName, - ).Return(collectionID) + mock.AnythingOfType("uint64"), + ).Return(&model.Collection{ + CollectionID: collectionID, + Name: oldName, + }, nil) meta.On("IsAlias", mock.Anything, newDB, newName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, newDB, newName, - ).Return(int64(0)) + mock.AnythingOfType("uint64"), + ).Return(nil, errors.New("not found")) meta.On("ListAliasesByID", mock.Anything, mock.Anything, @@ -668,21 +710,26 @@ func Test_renameCollectionTask_Execute(t *testing.T) { mock.Anything, oldName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, mock.Anything, oldName, - ).Return(collectionID) + mock.AnythingOfType("uint64"), + ).Return(&model.Collection{ + CollectionID: collectionID, + Name: oldName, + }, nil) meta.On("IsAlias", mock.Anything, mock.Anything, newName, ).Return(false) - meta.On("GetCollectionID", + meta.On("GetCollectionByName", mock.Anything, mock.Anything, newName, - ).Return(int64(0)) + mock.AnythingOfType("uint64"), + ).Return(nil, errors.New("not found")) meta.On("ListAliasesByID", mock.Anything, mock.Anything,