Skip to content

Commit 26b80fe

Browse files
yhmoyhmo
committed
feat: support to drop properties of field
Signed-off-by: yhmo <[email protected]>
1 parent 022d149 commit 26b80fe

File tree

8 files changed

+329
-28
lines changed

8 files changed

+329
-28
lines changed

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ require (
2121
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
2222
github.com/klauspost/compress v1.18.0
2323
github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d
24-
github.com/milvus-io/milvus-proto/go-api/v2 v2.5.12
24+
github.com/milvus-io/milvus-proto/go-api/v2 v2.5.13-0.20250520065018-13f9a20ffaad
2525
github.com/minio/minio-go/v7 v7.0.73
2626
github.com/panjf2000/ants/v2 v2.11.3 // indirect
2727
github.com/pingcap/log v1.1.1-0.20221015072633-39906604fb81

‎go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -634,8 +634,8 @@ github.com/milvus-io/cgosymbolizer v0.0.0-20240722103217-b7dee0e50119 h1:9VXijWu
634634
github.com/milvus-io/cgosymbolizer v0.0.0-20240722103217-b7dee0e50119/go.mod h1:DvXTE/K/RtHehxU8/GtDs4vFtfw64jJ3PaCnFri8CRg=
635635
github.com/milvus-io/gorocksdb v0.0.0-20220624081344-8c5f4212846b h1:TfeY0NxYxZzUfIfYe5qYDBzt4ZYRqzUjTR6CvUzjat8=
636636
github.com/milvus-io/gorocksdb v0.0.0-20220624081344-8c5f4212846b/go.mod h1:iwW+9cWfIzzDseEBCCeDSN5SD16Tidvy8cwQ7ZY8Qj4=
637-
github.com/milvus-io/milvus-proto/go-api/v2 v2.5.12 h1:zRMCf6W4kzBNqZb1CMmmfetUAG0y3hlT5Ow0H9MaASM=
638-
github.com/milvus-io/milvus-proto/go-api/v2 v2.5.12/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
637+
github.com/milvus-io/milvus-proto/go-api/v2 v2.5.13-0.20250520065018-13f9a20ffaad h1:tkSvshW0g1nbf7gPIsku838FFvrlucnPtAXYBiGuTAs=
638+
github.com/milvus-io/milvus-proto/go-api/v2 v2.5.13-0.20250520065018-13f9a20ffaad/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
639639
github.com/milvus-io/pulsar-client-go v0.12.1 h1:O2JZp1tsYiO7C0MQ4hrUY/aJXnn2Gry6hpm7UodghmE=
640640
github.com/milvus-io/pulsar-client-go v0.12.1/go.mod h1:dkutuH4oS2pXiGm+Ti7fQZ4MRjrMPZ8IJeEGAWMeckk=
641641
github.com/minio/asm2plan9s v0.0.0-20200509001527-cdd76441f9d8 h1:AMFGa4R4MiIpspGNG7Z948v4n35fFGB3RR3G/ry4FWs=

‎internal/proxy/task.go

Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,30 +1157,48 @@ const (
11571157
MmapEnabledKey = "mmap_enabled"
11581158
)
11591159

1160-
var allowedProps = []string{
1160+
var allowedAlterProps = []string{
11611161
common.MaxLengthKey,
11621162
common.MmapEnabledKey,
11631163
common.MaxCapacityKey,
11641164
}
11651165

1166-
func IsKeyAllowed(key string) bool {
1167-
for _, allowedKey := range allowedProps {
1166+
var allowedDropProps = []string{
1167+
common.MmapEnabledKey,
1168+
}
1169+
1170+
func IsKeyAllowAlter(key string) bool {
1171+
for _, allowedKey := range allowedAlterProps {
11681172
if key == allowedKey {
11691173
return true
11701174
}
11711175
}
11721176
return false
11731177
}
11741178

1179+
func IsKeyAllowDrop(key string) bool {
1180+
for _, allowedKey := range allowedDropProps {
1181+
if key == allowedKey {
1182+
return true
1183+
}
1184+
}
1185+
return false
1186+
}
1187+
1188+
func updateKey(key string) string {
1189+
var updatedKey string
1190+
if key == MmapEnabledKey {
1191+
updatedKey = common.MmapEnabledKey
1192+
} else {
1193+
updatedKey = key
1194+
}
1195+
return updatedKey
1196+
}
1197+
11751198
func updatePropertiesKeys(oldProps []*commonpb.KeyValuePair) []*commonpb.KeyValuePair {
11761199
props := make(map[string]string)
11771200
for _, prop := range oldProps {
1178-
var updatedKey string
1179-
if prop.Key == MmapEnabledKey {
1180-
updatedKey = common.MmapEnabledKey
1181-
} else {
1182-
updatedKey = prop.Key
1183-
}
1201+
updatedKey := updateKey(prop.Key)
11841202
props[updatedKey] = prop.Value
11851203
}
11861204

@@ -1200,22 +1218,31 @@ func (t *alterCollectionFieldTask) PreExecute(ctx context.Context) error {
12001218
if err != nil {
12011219
return err
12021220
}
1221+
1222+
isCollectionLoadedFn := func() (bool, error) {
1223+
collectionID, err := globalMetaCache.GetCollectionID(ctx, t.GetDbName(), t.CollectionName)
1224+
if err != nil {
1225+
return false, err
1226+
}
1227+
loaded, err1 := isCollectionLoaded(ctx, t.queryCoord, collectionID)
1228+
if err1 != nil {
1229+
return false, err1
1230+
}
1231+
return loaded, nil
1232+
}
1233+
12031234
t.Properties = updatePropertiesKeys(t.Properties)
12041235
for _, prop := range t.Properties {
1205-
if !IsKeyAllowed(prop.Key) {
1236+
if !IsKeyAllowAlter(prop.Key) {
12061237
return merr.WrapErrParameterInvalidMsg("%s does not allow update in collection field param", prop.Key)
12071238
}
12081239
// Check the value type based on the key
12091240
switch prop.Key {
12101241
case common.MmapEnabledKey:
1211-
collectionID, err := globalMetaCache.GetCollectionID(ctx, t.GetDbName(), t.CollectionName)
1242+
loaded, err := isCollectionLoadedFn()
12121243
if err != nil {
12131244
return err
12141245
}
1215-
loaded, err1 := isCollectionLoaded(ctx, t.queryCoord, collectionID)
1216-
if err1 != nil {
1217-
return err1
1218-
}
12191246
if loaded {
12201247
return merr.WrapErrCollectionLoaded(t.CollectionName, "can not alter collection field properties if collection loaded")
12211248
}
@@ -1266,6 +1293,27 @@ func (t *alterCollectionFieldTask) PreExecute(ctx context.Context) error {
12661293
}
12671294
}
12681295

1296+
deleteKeys := make([]string, 0)
1297+
for _, key := range t.DeleteKeys {
1298+
updatedKey := updateKey(key)
1299+
if !IsKeyAllowDrop(updatedKey) {
1300+
return merr.WrapErrParameterInvalidMsg("%s is not allowed to drop in collection field param", key)
1301+
}
1302+
1303+
if updatedKey == common.MmapEnabledKey {
1304+
loaded, err := isCollectionLoadedFn()
1305+
if err != nil {
1306+
return err
1307+
}
1308+
if loaded {
1309+
return merr.WrapErrCollectionLoaded(t.CollectionName, "can not drop collection field properties if collection loaded")
1310+
}
1311+
}
1312+
1313+
deleteKeys = append(deleteKeys, updatedKey)
1314+
}
1315+
t.DeleteKeys = deleteKeys
1316+
12691317
return nil
12701318
}
12711319

‎internal/proxy/task_test.go

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4445,6 +4445,8 @@ func TestAlterCollectionFieldCheckLoaded(t *testing.T) {
44454445
CollectionIDs: []int64{resp.CollectionID},
44464446
InMemoryPercentages: []int64{100},
44474447
}, nil)
4448+
4449+
// update property "mmap.enabled" but the collection is loaded
44484450
task := &alterCollectionFieldTask{
44494451
AlterCollectionFieldRequest: &milvuspb.AlterCollectionFieldRequest{
44504452
Base: &commonpb.MsgBase{},
@@ -4455,6 +4457,18 @@ func TestAlterCollectionFieldCheckLoaded(t *testing.T) {
44554457
}
44564458
err = task.PreExecute(context.Background())
44574459
assert.Equal(t, merr.Code(merr.ErrCollectionLoaded), merr.Code(err))
4460+
4461+
// delete property "mmap.enabled" but the collection is loaded
4462+
task = &alterCollectionFieldTask{
4463+
AlterCollectionFieldRequest: &milvuspb.AlterCollectionFieldRequest{
4464+
Base: &commonpb.MsgBase{},
4465+
CollectionName: collectionName,
4466+
DeleteKeys: []string{common.MmapEnabledKey},
4467+
},
4468+
queryCoord: qc,
4469+
}
4470+
err = task.PreExecute(context.Background())
4471+
assert.Equal(t, merr.Code(merr.ErrCollectionLoaded), merr.Code(err))
44584472
}
44594473

44604474
func TestAlterCollectionField1(t *testing.T) {
@@ -4498,18 +4512,25 @@ func TestAlterCollectionField1(t *testing.T) {
44984512
ShardsNum: 1,
44994513
}
45004514
rc.CreateCollection(context.Background(), createColReq)
4501-
resp, err := rc.DescribeCollection(context.Background(), &milvuspb.DescribeCollectionRequest{CollectionName: collectionName})
4502-
assert.NoError(t, err)
4515+
4516+
// The collection is not loaded
45034517
qc.EXPECT().ShowCollections(mock.Anything, mock.Anything).Return(&querypb.ShowCollectionsResponse{
45044518
Status: &commonpb.Status{ErrorCode: commonpb.ErrorCode_Success},
4505-
CollectionIDs: []int64{resp.CollectionID},
4506-
InMemoryPercentages: []int64{100},
4519+
CollectionIDs: []int64{},
4520+
InMemoryPercentages: []int64{},
45074521
}, nil)
4522+
45084523
// Test cases
4524+
// 1. the collection is not loaded, updating properties is allowed, deleting mmap.enabled/mmap_enabled is allowed
4525+
// 2. max_length can only be updated on varchar field or arrar field with varchar element
4526+
// 3. max_capacity can only be updated on array field
4527+
// 4. invalid number for max_length/max_capacity is not allowed
4528+
// 5. not allow to delete max_length/max_capacity
45094529
tests := []struct {
45104530
name string
45114531
fieldName string
45124532
properties []*commonpb.KeyValuePair
4533+
deleteKeys []string
45134534
expectError bool
45144535
errCode int32
45154536
}{
@@ -4592,6 +4613,41 @@ func TestAlterCollectionField1(t *testing.T) {
45924613
expectError: true,
45934614
errCode: merr.Code(merr.ErrParameterInvalid),
45944615
},
4616+
{
4617+
name: "not allow to update max_length on non-varchar field",
4618+
fieldName: "array_field",
4619+
properties: []*commonpb.KeyValuePair{
4620+
{Key: common.MaxLengthKey, Value: "10"},
4621+
},
4622+
expectError: true,
4623+
errCode: merr.Code(merr.ErrParameterInvalid),
4624+
},
4625+
{
4626+
name: "delete mmap.enabled is allowed",
4627+
fieldName: "string_field",
4628+
deleteKeys: []string{common.MmapEnabledKey},
4629+
expectError: false,
4630+
},
4631+
{
4632+
name: "delete mmap_enabled is allowed",
4633+
fieldName: "string_field",
4634+
deleteKeys: []string{MmapEnabledKey},
4635+
expectError: false,
4636+
},
4637+
{
4638+
name: "delete max_length is not allowed",
4639+
fieldName: "string_field",
4640+
deleteKeys: []string{common.MaxLengthKey},
4641+
expectError: true,
4642+
errCode: merr.Code(merr.ErrParameterInvalid),
4643+
},
4644+
{
4645+
name: "delete max_capacity is not allowed",
4646+
fieldName: "array_field",
4647+
deleteKeys: []string{common.MaxCapacityKey},
4648+
expectError: true,
4649+
errCode: merr.Code(merr.ErrParameterInvalid),
4650+
},
45954651
}
45964652

45974653
for _, test := range tests {
@@ -4602,6 +4658,7 @@ func TestAlterCollectionField1(t *testing.T) {
46024658
CollectionName: collectionName,
46034659
FieldName: test.fieldName,
46044660
Properties: test.properties,
4661+
DeleteKeys: test.deleteKeys,
46054662
},
46064663
queryCoord: qc,
46074664
}

‎internal/rootcoord/alter_collection_task.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,8 @@ func (a *alterCollectionFieldTask) Prepare(ctx context.Context) error {
274274
}
275275

276276
func (a *alterCollectionFieldTask) Execute(ctx context.Context) error {
277-
if a.Req.GetProperties() == nil {
278-
return errors.New("only support alter collection properties, but collection field properties is empty")
277+
if len(a.Req.GetProperties()) == 0 && len(a.Req.GetDeleteKeys()) == 0 {
278+
return errors.New("The field properties to alter and keys to delete must not be empty at the same time")
279279
}
280280

281281
oldColl, err := a.core.meta.GetCollectionByName(ctx, a.Req.GetDbName(), a.Req.GetCollectionName(), a.ts)
@@ -314,7 +314,13 @@ func executeAlterCollectionFieldTaskSteps(ctx context.Context,
314314
) error {
315315
var err error
316316
filedName := request.GetFieldName()
317-
newFieldProperties := UpdateFieldPropertyParams(oldFieldProperties, request.GetProperties())
317+
318+
var newFieldProperties []*commonpb.KeyValuePair
319+
if len(request.Properties) > 0 {
320+
newFieldProperties = UpdateFieldPropertyParams(oldFieldProperties, request.GetProperties())
321+
} else if len(request.DeleteKeys) > 0 {
322+
newFieldProperties = DeleteProperties(oldFieldProperties, request.GetDeleteKeys())
323+
}
318324
oldColl := col.Clone()
319325
err = ResetFieldProperties(oldColl, filedName, oldFieldProperties)
320326
if err != nil {

0 commit comments

Comments
 (0)