Skip to content

Commit d1b40b7

Browse files
XuanYang-cnXuanYang-cn
authored
fix: Rename Collection use wrong method to check if collection exists (#44436)
See also: #44435 Signed-off-by: yangxuan <xuan.yang@zilliz.com>
1 parent ba28989 commit d1b40b7

File tree

2 files changed

+88
-39
lines changed

2 files changed

+88
-39
lines changed

‎internal/rootcoord/rename_collection_task.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/milvus-io/milvus/internal/util/hookutil"
2626
"github.com/milvus-io/milvus/internal/util/proxyutil"
2727
"github.com/milvus-io/milvus/pkg/v2/util"
28+
"github.com/milvus-io/milvus/pkg/v2/util/typeutil"
2829
)
2930

3031
type renameCollectionTask struct {
@@ -63,13 +64,13 @@ func (t *renameCollectionTask) Execute(ctx context.Context) error {
6364
return fmt.Errorf("unsupported use an alias to rename collection, alias:%s", t.Req.GetOldName())
6465
}
6566

66-
collID := t.core.meta.GetCollectionID(ctx, t.Req.GetDbName(), t.Req.GetOldName())
67-
if collID == 0 {
67+
collInfo, err := t.core.meta.GetCollectionByName(ctx, t.Req.GetDbName(), t.Req.GetOldName(), typeutil.MaxTimestamp)
68+
if err != nil {
6869
return fmt.Errorf("collection not found in database, collection: %s, database: %s", t.Req.GetOldName(), t.Req.GetDbName())
6970
}
7071

7172
// check old collection doesn't have aliases if renaming databases
72-
aliases := t.core.meta.ListAliasesByID(ctx, collID)
73+
aliases := t.core.meta.ListAliasesByID(ctx, collInfo.CollectionID)
7374
if len(aliases) > 0 && targetDB != t.Req.GetDbName() {
7475
return fmt.Errorf("fail to rename collection to different database, must drop all aliases of collection %s before rename", t.Req.GetOldName())
7576
}
@@ -79,7 +80,8 @@ func (t *renameCollectionTask) Execute(ctx context.Context) error {
7980
return fmt.Errorf("cannot rename collection to an existing alias: %s", t.Req.GetNewName())
8081
}
8182

82-
if existingCollID := t.core.meta.GetCollectionID(ctx, targetDB, t.Req.GetNewName()); existingCollID != 0 {
83+
_, err = t.core.meta.GetCollectionByName(ctx, targetDB, t.Req.GetNewName(), typeutil.MaxTimestamp)
84+
if err == nil {
8385
return fmt.Errorf("duplicated new collection name %s in database %s with other collection name or alias", t.Req.GetNewName(), targetDB)
8486
}
8587

@@ -101,7 +103,7 @@ func (t *renameCollectionTask) Execute(ctx context.Context) error {
101103
baseStep: baseStep{core: t.core},
102104
dbName: t.Req.GetDbName(),
103105
collectionNames: append(aliases, t.Req.GetOldName()),
104-
collectionID: collID,
106+
collectionID: collInfo.CollectionID,
105107
ts: ts,
106108
opts: []proxyutil.ExpireCacheOpt{proxyutil.SetMsgType(commonpb.MsgType_RenameCollection)},
107109
})

‎internal/rootcoord/rename_collection_task_test.go‎

Lines changed: 81 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -134,11 +134,15 @@ func Test_renameCollectionTask_Prepare(t *testing.T) {
134134
"db1",
135135
"old_collection",
136136
).Return(false)
137-
meta.On("GetCollectionID",
137+
meta.On("GetCollectionByName",
138138
mock.Anything,
139139
"db1",
140140
"old_collection",
141-
).Return(collectionID)
141+
mock.AnythingOfType("uint64"),
142+
).Return(&model.Collection{
143+
CollectionID: collectionID,
144+
Name: "old_collection",
145+
}, nil)
142146
meta.On("ListAliasesByID",
143147
mock.Anything,
144148
collectionID,
@@ -148,11 +152,12 @@ func Test_renameCollectionTask_Prepare(t *testing.T) {
148152
"db2",
149153
"new_collection",
150154
).Return(false)
151-
meta.On("GetCollectionID",
155+
meta.On("GetCollectionByName",
152156
mock.Anything,
153157
"db2",
154158
"new_collection",
155-
).Return(int64(0))
159+
mock.AnythingOfType("uint64"),
160+
).Return(nil, errors.New("not found"))
156161
meta.On("RenameCollection",
157162
mock.Anything,
158163
"db1",
@@ -195,11 +200,12 @@ func Test_renameCollectionTask_Execute(t *testing.T) {
195200
mock.Anything,
196201
oldName,
197202
).Return(false)
198-
meta.On("GetCollectionID",
203+
meta.On("GetCollectionByName",
199204
mock.Anything,
200205
mock.Anything,
201206
oldName,
202-
).Return(int64(0))
207+
mock.AnythingOfType("uint64"),
208+
).Return(nil, errors.New("collection not found"))
203209

204210
core := newTestCore(withMeta(meta))
205211
task := &renameCollectionTask{
@@ -230,11 +236,15 @@ func Test_renameCollectionTask_Execute(t *testing.T) {
230236
mock.Anything,
231237
oldName,
232238
).Return(false)
233-
meta.On("GetCollectionID",
239+
meta.On("GetCollectionByName",
234240
mock.Anything,
235241
mock.Anything,
236242
oldName,
237-
).Return(collectionID)
243+
mock.AnythingOfType("uint64"),
244+
).Return(&model.Collection{
245+
CollectionID: collectionID,
246+
Name: oldName,
247+
}, nil)
238248
meta.On("ListAliasesByID",
239249
mock.Anything,
240250
collectionID,
@@ -244,11 +254,12 @@ func Test_renameCollectionTask_Execute(t *testing.T) {
244254
mock.Anything,
245255
newName,
246256
).Return(false)
247-
meta.On("GetCollectionID",
257+
meta.On("GetCollectionByName",
248258
mock.Anything,
249259
mock.Anything,
250260
newName,
251-
).Return(int64(0))
261+
mock.AnythingOfType("uint64"),
262+
).Return(nil, errors.New("not found"))
252263
meta.On("RenameCollection",
253264
mock.Anything,
254265
mock.Anything,
@@ -287,11 +298,15 @@ func Test_renameCollectionTask_Execute(t *testing.T) {
287298
mock.Anything,
288299
oldName,
289300
).Return(false)
290-
meta.On("GetCollectionID",
301+
meta.On("GetCollectionByName",
291302
mock.Anything,
292303
mock.Anything,
293304
oldName,
294-
).Return(collectionID)
305+
mock.AnythingOfType("uint64"),
306+
).Return(&model.Collection{
307+
CollectionID: collectionID,
308+
Name: oldName,
309+
}, nil)
295310
meta.On("ListAliasesByID",
296311
mock.Anything,
297312
collectionID,
@@ -301,11 +316,12 @@ func Test_renameCollectionTask_Execute(t *testing.T) {
301316
mock.Anything,
302317
newName,
303318
).Return(false)
304-
meta.On("GetCollectionID",
319+
meta.On("GetCollectionByName",
305320
mock.Anything,
306321
mock.Anything,
307322
newName,
308-
).Return(int64(0))
323+
mock.AnythingOfType("uint64"),
324+
).Return(nil, errors.New("not found"))
309325
meta.On("RenameCollection",
310326
mock.Anything,
311327
mock.Anything,
@@ -345,21 +361,26 @@ func Test_renameCollectionTask_Execute(t *testing.T) {
345361
mock.Anything,
346362
oldName,
347363
).Return(false)
348-
meta.On("GetCollectionID",
364+
meta.On("GetCollectionByName",
349365
mock.Anything,
350366
mock.Anything,
351367
oldName,
352-
).Return(collectionID)
368+
mock.AnythingOfType("uint64"),
369+
).Return(&model.Collection{
370+
CollectionID: collectionID,
371+
Name: oldName,
372+
}, nil)
353373
meta.On("IsAlias",
354374
mock.Anything,
355375
mock.Anything,
356376
newName,
357377
).Return(false)
358-
meta.On("GetCollectionID",
378+
meta.On("GetCollectionByName",
359379
mock.Anything,
360380
mock.Anything,
361381
newName,
362-
).Return(int64(0))
382+
mock.AnythingOfType("uint64"),
383+
).Return(nil, errors.New("not found"))
363384
meta.On("ListAliasesByID",
364385
mock.Anything,
365386
mock.Anything,
@@ -425,11 +446,15 @@ func Test_renameCollectionTask_Execute(t *testing.T) {
425446
"db1",
426447
oldName,
427448
).Return(false)
428-
meta.On("GetCollectionID",
449+
meta.On("GetCollectionByName",
429450
mock.Anything,
430451
"db1",
431452
oldName,
432-
).Return(collectionID)
453+
mock.AnythingOfType("uint64"),
454+
).Return(&model.Collection{
455+
CollectionID: collectionID,
456+
Name: oldName,
457+
}, nil)
433458
meta.On("ListAliasesByID",
434459
mock.Anything,
435460
collectionID,
@@ -497,11 +522,15 @@ func Test_renameCollectionTask_Execute(t *testing.T) {
497522
mock.Anything,
498523
oldName,
499524
).Return(false)
500-
meta.On("GetCollectionID",
525+
meta.On("GetCollectionByName",
501526
mock.Anything,
502527
mock.Anything,
503528
oldName,
504-
).Return(collectionID)
529+
mock.AnythingOfType("uint64"),
530+
).Return(&model.Collection{
531+
CollectionID: collectionID,
532+
Name: oldName,
533+
}, nil)
505534
meta.On("ListAliasesByID",
506535
mock.Anything,
507536
collectionID,
@@ -542,11 +571,15 @@ func Test_renameCollectionTask_Execute(t *testing.T) {
542571
mock.Anything,
543572
oldName,
544573
).Return(false)
545-
meta.On("GetCollectionID",
574+
meta.On("GetCollectionByName",
546575
mock.Anything,
547576
mock.Anything,
548577
oldName,
549-
).Return(collectionID)
578+
mock.AnythingOfType("uint64"),
579+
).Return(&model.Collection{
580+
CollectionID: collectionID,
581+
Name: oldName,
582+
}, nil)
550583
meta.On("ListAliasesByID",
551584
mock.Anything,
552585
collectionID,
@@ -556,11 +589,15 @@ func Test_renameCollectionTask_Execute(t *testing.T) {
556589
mock.Anything,
557590
newName,
558591
).Return(false)
559-
meta.On("GetCollectionID",
592+
meta.On("GetCollectionByName",
560593
mock.Anything,
561594
mock.Anything,
562595
newName,
563-
).Return(existingCollID)
596+
mock.AnythingOfType("uint64"),
597+
).Return(&model.Collection{
598+
CollectionID: existingCollID,
599+
Name: newName,
600+
}, nil)
564601

565602
core := newTestCore(withMeta(meta))
566603
task := &renameCollectionTask{
@@ -612,21 +649,26 @@ func Test_renameCollectionTask_Execute(t *testing.T) {
612649
oldDB,
613650
oldName,
614651
).Return(false)
615-
meta.On("GetCollectionID",
652+
meta.On("GetCollectionByName",
616653
mock.Anything,
617654
oldDB,
618655
oldName,
619-
).Return(collectionID)
656+
mock.AnythingOfType("uint64"),
657+
).Return(&model.Collection{
658+
CollectionID: collectionID,
659+
Name: oldName,
660+
}, nil)
620661
meta.On("IsAlias",
621662
mock.Anything,
622663
newDB,
623664
newName,
624665
).Return(false)
625-
meta.On("GetCollectionID",
666+
meta.On("GetCollectionByName",
626667
mock.Anything,
627668
newDB,
628669
newName,
629-
).Return(int64(0))
670+
mock.AnythingOfType("uint64"),
671+
).Return(nil, errors.New("not found"))
630672
meta.On("ListAliasesByID",
631673
mock.Anything,
632674
mock.Anything,
@@ -668,21 +710,26 @@ func Test_renameCollectionTask_Execute(t *testing.T) {
668710
mock.Anything,
669711
oldName,
670712
).Return(false)
671-
meta.On("GetCollectionID",
713+
meta.On("GetCollectionByName",
672714
mock.Anything,
673715
mock.Anything,
674716
oldName,
675-
).Return(collectionID)
717+
mock.AnythingOfType("uint64"),
718+
).Return(&model.Collection{
719+
CollectionID: collectionID,
720+
Name: oldName,
721+
}, nil)
676722
meta.On("IsAlias",
677723
mock.Anything,
678724
mock.Anything,
679725
newName,
680726
).Return(false)
681-
meta.On("GetCollectionID",
727+
meta.On("GetCollectionByName",
682728
mock.Anything,
683729
mock.Anything,
684730
newName,
685-
).Return(int64(0))
731+
mock.AnythingOfType("uint64"),
732+
).Return(nil, errors.New("not found"))
686733
meta.On("ListAliasesByID",
687734
mock.Anything,
688735
mock.Anything,

0 commit comments

Comments
 (0)