diff --git a/internal/proxy/task.go b/internal/proxy/task.go index 7ed10526b7..ccad9755da 100644 --- a/internal/proxy/task.go +++ b/internal/proxy/task.go @@ -1269,6 +1269,7 @@ const ( var allowedProps = []string{ common.MaxLengthKey, common.MmapEnabledKey, + common.MaxCapacityKey, } func IsKeyAllowed(key string) bool { @@ -1331,26 +1332,46 @@ func (t *alterCollectionFieldTask) PreExecute(ctx context.Context) error { case common.MaxLengthKey: IsStringType := false fieldName := "" - var dataType int32 for _, field := range collSchema.Fields { if field.GetName() == t.FieldName && (typeutil.IsStringType(field.DataType) || typeutil.IsArrayContainStringElementType(field.DataType, field.ElementType)) { IsStringType = true fieldName = field.GetName() - dataType = int32(field.DataType) + break } } if !IsStringType { - return merr.WrapErrParameterInvalid(fieldName, "%s can not modify the maxlength for non-string types", schemapb.DataType_name[dataType]) + return merr.WrapErrParameterInvalidMsg("%s can not modify the maxlength for non-string types", fieldName) } value, err := strconv.Atoi(prop.Value) if err != nil { - return merr.WrapErrParameterInvalid("%s should be an integer, but got %T", prop.Key, prop.Value) + return merr.WrapErrParameterInvalidMsg("%s should be an integer, but got %T", prop.Key, prop.Value) } defaultMaxVarCharLength := Params.ProxyCfg.MaxVarCharLength.GetAsInt64() if int64(value) > defaultMaxVarCharLength { return merr.WrapErrParameterInvalidMsg("%s exceeds the maximum allowed value %s", prop.Value, strconv.FormatInt(defaultMaxVarCharLength, 10)) } + case common.MaxCapacityKey: + IsArrayType := false + fieldName := "" + for _, field := range collSchema.Fields { + if field.GetName() == t.FieldName && typeutil.IsArrayType(field.DataType) { + IsArrayType = true + fieldName = field.GetName() + break + } + } + if !IsArrayType { + return merr.WrapErrParameterInvalidMsg("%s can not modify the maxcapacity for non-array types", fieldName) + } + + maxCapacityPerRow, err := strconv.ParseInt(prop.Value, 10, 64) + if err != nil { + return merr.WrapErrParameterInvalidMsg("the value for %s of field %s must be an integer", common.MaxCapacityKey, fieldName) + } + if maxCapacityPerRow > defaultMaxArrayCapacity || maxCapacityPerRow <= 0 { + return merr.WrapErrParameterInvalidMsg("the maximum capacity specified for a Array should be in (0, %d]", defaultMaxArrayCapacity) + } } } diff --git a/internal/proxy/task_test.go b/internal/proxy/task_test.go index c725a13fdc..9db3ea14da 100644 --- a/internal/proxy/task_test.go +++ b/internal/proxy/task_test.go @@ -4621,3 +4621,206 @@ func TestInsertForReplicate(t *testing.T) { assert.Error(t, err) }) } + +func TestAlterCollectionFieldCheckLoaded(t *testing.T) { + qc := NewMixCoordMock() + InitMetaCache(context.Background(), qc, nil) + collectionName := "test_alter_collection_field_check_loaded" + createColReq := &milvuspb.CreateCollectionRequest{ + Base: &commonpb.MsgBase{ + MsgType: commonpb.MsgType_DropCollection, + MsgID: 100, + Timestamp: 100, + }, + DbName: dbName, + CollectionName: collectionName, + Schema: nil, + ShardsNum: 1, + } + qc.CreateCollection(context.Background(), createColReq) + resp, err := qc.DescribeCollection(context.Background(), &milvuspb.DescribeCollectionRequest{CollectionName: collectionName}) + assert.NoError(t, err) + + qc.ShowLoadCollectionsFunc = func(ctx context.Context, req *querypb.ShowCollectionsRequest, opts ...grpc.CallOption) (*querypb.ShowCollectionsResponse, error) { + return &querypb.ShowCollectionsResponse{ + Status: &commonpb.Status{ + ErrorCode: commonpb.ErrorCode_Success, + }, + CollectionIDs: []int64{resp.CollectionID}, + InMemoryPercentages: []int64{100}, + }, nil + } + + task := &alterCollectionFieldTask{ + AlterCollectionFieldRequest: &milvuspb.AlterCollectionFieldRequest{ + Base: &commonpb.MsgBase{}, + CollectionName: collectionName, + Properties: []*commonpb.KeyValuePair{{Key: common.MmapEnabledKey, Value: "true"}}, + }, + mixCoord: qc, + } + err = task.PreExecute(context.Background()) + assert.Equal(t, merr.Code(merr.ErrCollectionLoaded), merr.Code(err)) +} + +func TestAlterCollectionField(t *testing.T) { + qc := NewMixCoordMock() + InitMetaCache(context.Background(), qc, nil) + collectionName := "test_alter_collection_field" + + // Create collection with string and array fields + schema := &schemapb.CollectionSchema{ + Name: collectionName, + Fields: []*schemapb.FieldSchema{ + { + FieldID: 100, + Name: "string_field", + DataType: schemapb.DataType_VarChar, + TypeParams: []*commonpb.KeyValuePair{{Key: "max_length", Value: "100"}}, + }, + { + FieldID: 101, + Name: "array_field", + DataType: schemapb.DataType_Array, + ElementType: schemapb.DataType_Int64, + TypeParams: []*commonpb.KeyValuePair{{Key: "max_capacity", Value: "100"}}, + }, + }, + } + schemaBytes, err := proto.Marshal(schema) + assert.NoError(t, err) + + createColReq := &milvuspb.CreateCollectionRequest{ + Base: &commonpb.MsgBase{ + MsgType: commonpb.MsgType_CreateCollection, + MsgID: 100, + Timestamp: 100, + }, + DbName: dbName, + CollectionName: collectionName, + Schema: schemaBytes, + ShardsNum: 1, + } + qc.CreateCollection(context.Background(), createColReq) + + // Test cases + tests := []struct { + name string + fieldName string + properties []*commonpb.KeyValuePair + expectError bool + errCode int32 + }{ + { + name: "update string field max_length", + fieldName: "string_field", + properties: []*commonpb.KeyValuePair{ + {Key: common.MaxLengthKey, Value: "200"}, + }, + expectError: false, + }, + { + name: "update array field max_capacity", + fieldName: "array_field", + properties: []*commonpb.KeyValuePair{ + {Key: common.MaxCapacityKey, Value: "200"}, + }, + expectError: false, + }, + { + name: "update mmap_enabled", + fieldName: "string_field", + properties: []*commonpb.KeyValuePair{ + {Key: common.MmapEnabledKey, Value: "true"}, + }, + expectError: false, + }, + { + name: "invalid property key", + fieldName: "string_field", + properties: []*commonpb.KeyValuePair{ + {Key: "invalid_key", Value: "value"}, + }, + expectError: true, + errCode: merr.Code(merr.ErrParameterInvalid), + }, + { + name: "invalid max_length value type", + fieldName: "string_field", + properties: []*commonpb.KeyValuePair{ + {Key: common.MaxLengthKey, Value: "not_a_number"}, + }, + expectError: true, + errCode: merr.Code(merr.ErrParameterInvalid), + }, + { + name: "invalid max_capacity value type", + fieldName: "array_field", + properties: []*commonpb.KeyValuePair{ + {Key: common.MaxCapacityKey, Value: "not_a_number"}, + }, + expectError: true, + errCode: merr.Code(merr.ErrParameterInvalid), + }, + { + name: "max_length exceeds limit", + fieldName: "string_field", + properties: []*commonpb.KeyValuePair{ + {Key: common.MaxLengthKey, Value: "65536"}, + }, + expectError: true, + errCode: merr.Code(merr.ErrParameterInvalid), + }, + { + name: "max_capacity exceeds limit", + fieldName: "array_field", + properties: []*commonpb.KeyValuePair{ + {Key: common.MaxCapacityKey, Value: "5000"}, + }, + expectError: true, + errCode: merr.Code(merr.ErrParameterInvalid), + }, + { + name: "max_capacity invalid range", + fieldName: "array_field", + properties: []*commonpb.KeyValuePair{ + {Key: common.MaxCapacityKey, Value: "0"}, + }, + expectError: true, + errCode: merr.Code(merr.ErrParameterInvalid), + }, + { + name: "max_capacity invalid type", + fieldName: "string_field", + properties: []*commonpb.KeyValuePair{ + {Key: common.MaxCapacityKey, Value: "3"}, + }, + expectError: true, + errCode: merr.Code(merr.ErrParameterInvalid), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + task := &alterCollectionFieldTask{ + AlterCollectionFieldRequest: &milvuspb.AlterCollectionFieldRequest{ + Base: &commonpb.MsgBase{}, + CollectionName: collectionName, + FieldName: test.fieldName, + Properties: test.properties, + }, + mixCoord: qc, + } + + err := task.PreExecute(context.Background()) + if test.expectError { + assert.Error(t, err) + if test.errCode != 0 { + assert.Equal(t, test.errCode, merr.Code(err)) + } + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/tests/python_client/milvus_client/test_milvus_client_alter.py b/tests/python_client/milvus_client/test_milvus_client_alter.py index e8c3e22110..0bccf8dedf 100644 --- a/tests/python_client/milvus_client/test_milvus_client_alter.py +++ b/tests/python_client/milvus_client/test_milvus_client_alter.py @@ -228,14 +228,12 @@ class TestMilvusClientAlterCollectionField(TestMilvusClientV2Base): field_params={"mmap.enabled": False}) self.alter_collection_field(client, collection_name, field_name=array_field_name, field_params={"max_length": new_max_length}) + self.alter_collection_field(client, collection_name, field_name=array_field_name, + field_params={"max_capacity": 20}) error = {ct.err_code: 999, ct.err_msg: f"can not modify the maxlength for non-string types"} self.alter_collection_field(client, collection_name, field_name=vector_field_name, field_params={"max_length": new_max_length}, check_task=CheckTasks.err_res, check_items=error) - error = {ct.err_code: 999, ct.err_msg: "max_capacity does not allow update in collection field param"} - self.alter_collection_field(client, collection_name, field_name=array_field_name, - field_params={"max_capacity": 20}, - check_task=CheckTasks.err_res, check_items=error) error = {ct.err_code: 999, ct.err_msg: "element_type does not allow update in collection field param"} self.alter_collection_field(client, collection_name, field_name=array_field_name, field_params={"element_type": DataType.INT64}, @@ -244,7 +242,7 @@ class TestMilvusClientAlterCollectionField(TestMilvusClientV2Base): check_items={str_field_name: {"max_length": new_max_length, "mmap_enabled": False}, vector_field_name: {"mmap_enabled": False}, json_field_name: {"mmap_enabled": True}, - array_field_name: {"max_length": new_max_length, "max_capacity": 10}}) + array_field_name: {"max_length": new_max_length, "max_capacity": 20}}) # verify that cannot insert data with the old max_length for alter_field in [pk_field_name, str_field_name, array_field_name]: