Skip to content

Commit a48001f

Browse files
yhmoyhmo
authored
feat: support to drop properties of field (#41954)
issue: #41990 pr: #41996 Signed-off-by: yhmo <[email protected]>
1 parent f4e4bb2 commit a48001f

File tree

12 files changed

+335
-34
lines changed

12 files changed

+335
-34
lines changed

‎client/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ require (
66
github.com/blang/semver/v4 v4.0.0
77
github.com/cockroachdb/errors v1.9.1
88
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
9-
github.com/milvus-io/milvus-proto/go-api/v2 v2.5.12
9+
github.com/milvus-io/milvus-proto/go-api/v2 v2.5.13-0.20250520065018-13f9a20ffaad
1010
github.com/milvus-io/milvus/pkg/v2 v2.5.7
1111
github.com/quasilyte/go-ruleguard/dsl v0.3.22
1212
github.com/samber/lo v1.27.0

‎client/go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,8 +318,8 @@ github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfr
318318
github.com/mediocregopher/radix/v3 v3.4.2/go.mod h1:8FL3F6UQRXHXIBSPUs5h0RybMF8i4n7wVopoX3x7Bv8=
319319
github.com/microcosm-cc/bluemonday v1.0.2/go.mod h1:iVP4YcDBq+n/5fb23BhYFvIMq/leAFZyRl6bYmGDlGc=
320320
github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg=
321-
github.com/milvus-io/milvus-proto/go-api/v2 v2.5.12 h1:zRMCf6W4kzBNqZb1CMmmfetUAG0y3hlT5Ow0H9MaASM=
322-
github.com/milvus-io/milvus-proto/go-api/v2 v2.5.12/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
321+
github.com/milvus-io/milvus-proto/go-api/v2 v2.5.13-0.20250520065018-13f9a20ffaad h1:tkSvshW0g1nbf7gPIsku838FFvrlucnPtAXYBiGuTAs=
322+
github.com/milvus-io/milvus-proto/go-api/v2 v2.5.13-0.20250520065018-13f9a20ffaad/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
323323
github.com/milvus-io/milvus/pkg/v2 v2.5.7 h1:b45jq1s1v03AekFucs2/dkkXohB57gEx7gspJuAkfbY=
324324
github.com/milvus-io/milvus/pkg/v2 v2.5.7/go.mod h1:pImw1IGNS7k/5yvlZV2tZi5vZu1VQRlQij+r39d+XnI=
325325
github.com/mitchellh/cli v1.0.0/go.mod h1:hNIlj7HEI86fIcpObd7a0FcrxTWetlwJDGcceTlRvqc=

‎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
@@ -4451,6 +4451,8 @@ func TestAlterCollectionFieldCheckLoaded(t *testing.T) {
44514451
CollectionIDs: []int64{resp.CollectionID},
44524452
InMemoryPercentages: []int64{100},
44534453
}, nil)
4454+
4455+
// update property "mmap.enabled" but the collection is loaded
44544456
task := &alterCollectionFieldTask{
44554457
AlterCollectionFieldRequest: &milvuspb.AlterCollectionFieldRequest{
44564458
Base: &commonpb.MsgBase{},
@@ -4461,6 +4463,18 @@ func TestAlterCollectionFieldCheckLoaded(t *testing.T) {
44614463
}
44624464
err = task.PreExecute(context.Background())
44634465
assert.Equal(t, merr.Code(merr.ErrCollectionLoaded), merr.Code(err))
4466+
4467+
// delete property "mmap.enabled" but the collection is loaded
4468+
task = &alterCollectionFieldTask{
4469+
AlterCollectionFieldRequest: &milvuspb.AlterCollectionFieldRequest{
4470+
Base: &commonpb.MsgBase{},
4471+
CollectionName: collectionName,
4472+
DeleteKeys: []string{common.MmapEnabledKey},
4473+
},
4474+
queryCoord: qc,
4475+
}
4476+
err = task.PreExecute(context.Background())
4477+
assert.Equal(t, merr.Code(merr.ErrCollectionLoaded), merr.Code(err))
44644478
}
44654479

44664480
func TestAlterCollectionField1(t *testing.T) {
@@ -4504,18 +4518,25 @@ func TestAlterCollectionField1(t *testing.T) {
45044518
ShardsNum: 1,
45054519
}
45064520
rc.CreateCollection(context.Background(), createColReq)
4507-
resp, err := rc.DescribeCollection(context.Background(), &milvuspb.DescribeCollectionRequest{CollectionName: collectionName})
4508-
assert.NoError(t, err)
4521+
4522+
// The collection is not loaded
45094523
qc.EXPECT().ShowCollections(mock.Anything, mock.Anything).Return(&querypb.ShowCollectionsResponse{
45104524
Status: &commonpb.Status{ErrorCode: commonpb.ErrorCode_Success},
4511-
CollectionIDs: []int64{resp.CollectionID},
4512-
InMemoryPercentages: []int64{100},
4525+
CollectionIDs: []int64{},
4526+
InMemoryPercentages: []int64{},
45134527
}, nil)
4528+
45144529
// Test cases
4530+
// 1. the collection is not loaded, updating properties is allowed, deleting mmap.enabled/mmap_enabled is allowed
4531+
// 2. max_length can only be updated on varchar field or arrar field with varchar element
4532+
// 3. max_capacity can only be updated on array field
4533+
// 4. invalid number for max_length/max_capacity is not allowed
4534+
// 5. not allow to delete max_length/max_capacity
45154535
tests := []struct {
45164536
name string
45174537
fieldName string
45184538
properties []*commonpb.KeyValuePair
4539+
deleteKeys []string
45194540
expectError bool
45204541
errCode int32
45214542
}{
@@ -4598,6 +4619,41 @@ func TestAlterCollectionField1(t *testing.T) {
45984619
expectError: true,
45994620
errCode: merr.Code(merr.ErrParameterInvalid),
46004621
},
4622+
{
4623+
name: "not allow to update max_length on non-varchar field",
4624+
fieldName: "array_field",
4625+
properties: []*commonpb.KeyValuePair{
4626+
{Key: common.MaxLengthKey, Value: "10"},
4627+
},
4628+
expectError: true,
4629+
errCode: merr.Code(merr.ErrParameterInvalid),
4630+
},
4631+
{
4632+
name: "delete mmap.enabled is allowed",
4633+
fieldName: "string_field",
4634+
deleteKeys: []string{common.MmapEnabledKey},
4635+
expectError: false,
4636+
},
4637+
{
4638+
name: "delete mmap_enabled is allowed",
4639+
fieldName: "string_field",
4640+
deleteKeys: []string{MmapEnabledKey},
4641+
expectError: false,
4642+
},
4643+
{
4644+
name: "delete max_length is not allowed",
4645+
fieldName: "string_field",
4646+
deleteKeys: []string{common.MaxLengthKey},
4647+
expectError: true,
4648+
errCode: merr.Code(merr.ErrParameterInvalid),
4649+
},
4650+
{
4651+
name: "delete max_capacity is not allowed",
4652+
fieldName: "array_field",
4653+
deleteKeys: []string{common.MaxCapacityKey},
4654+
expectError: true,
4655+
errCode: merr.Code(merr.ErrParameterInvalid),
4656+
},
46014657
}
46024658

46034659
for _, test := range tests {
@@ -4608,6 +4664,7 @@ func TestAlterCollectionField1(t *testing.T) {
46084664
CollectionName: collectionName,
46094665
FieldName: test.fieldName,
46104666
Properties: test.properties,
4667+
DeleteKeys: test.deleteKeys,
46114668
},
46124669
queryCoord: qc,
46134670
}

‎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)