diff --git a/internal/proxy/task.go b/internal/proxy/task.go index e0734354b1..05a043ec3e 100644 --- a/internal/proxy/task.go +++ b/internal/proxy/task.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "math" + "strconv" "time" "github.com/cockroachdb/errors" @@ -1019,7 +1020,7 @@ func (t *alterCollectionTask) PreExecute(ctx context.Context) error { return err } if loaded { - return merr.WrapErrCollectionLoaded(" %s %s can not delete mmap properties if collection loaded", t.CollectionName, key) + return merr.WrapErrCollectionLoaded(t.CollectionName, "can not delete mmap properties if collection loaded") } } } @@ -1145,24 +1146,67 @@ func (t *alterCollectionFieldTask) OnEnqueue() error { return nil } +var allowedProps = []string{ + common.MaxLengthKey, + common.MmapEnabledKey, +} + +func IsKeyAllowed(key string) bool { + for _, allowedKey := range allowedProps { + if key == allowedKey { + return true + } + } + return false +} + func (t *alterCollectionFieldTask) PreExecute(ctx context.Context) error { collSchema, err := globalMetaCache.GetCollectionSchema(ctx, t.GetDbName(), t.CollectionName) if err != nil { return err } - 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) + for _, prop := range t.Properties { + if !IsKeyAllowed(prop.Key) { + return merr.WrapErrParameterInvalidMsg("%s does not allow update in collection field param", prop.Key) + } + // Check the value type based on the key + switch prop.Key { + case common.MmapEnabledKey: + collectionID, err := globalMetaCache.GetCollectionID(ctx, t.GetDbName(), t.CollectionName) + if err != nil { + return err + } + loaded, err1 := isCollectionLoaded(ctx, t.queryCoord, collectionID) + if err1 != nil { + return err1 + } + if loaded { + return merr.WrapErrCollectionLoaded(t.CollectionName, "can not alter collection field properties if collection loaded") + } + 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) + } + } + if !IsStringType { + return merr.WrapErrParameterInvalid(fieldName, "%s can not modify the maxlength for non-string types", schemapb.DataType_name[dataType]) + } + value, err := strconv.Atoi(prop.Value) + if err != nil { + return merr.WrapErrParameterInvalid("%s should be an integer, but got %T", prop.Key, prop.Value) + } + + if value > defaultMaxVarCharLength { + return merr.WrapErrParameterInvalid("%s exceeds the maximum allowed value 65535", prop.Value) + } } - } - if !IsStringType { - return merr.WrapErrParameterInvalid(fieldName, "%s can not modify the maxlength for non-string types", schemapb.DataType_name[dataType]) } return nil diff --git a/internal/proxy/task_index.go b/internal/proxy/task_index.go index 6fc646e39e..125b301bb2 100644 --- a/internal/proxy/task_index.go +++ b/internal/proxy/task_index.go @@ -619,13 +619,13 @@ func (t *alterIndexTask) PreExecute(ctx context.Context) error { if len(t.req.GetExtraParams()) > 0 { for _, param := range t.req.GetExtraParams() { if !indexparams.IsConfigableIndexParam(param.GetKey()) { - return merr.WrapErrParameterInvalidMsg("%s is not configable index param in extraParams", param.GetKey()) + return merr.WrapErrParameterInvalidMsg("%s is not a configable index proptery", param.GetKey()) } } } else if len(t.req.GetDeleteKeys()) > 0 { for _, param := range t.req.GetDeleteKeys() { if !indexparams.IsConfigableIndexParam(param) { - return merr.WrapErrParameterInvalidMsg("%s is not configable index param in deleteKeys", param) + return merr.WrapErrParameterInvalidMsg("%s is not a configable index proptery", param) } } } diff --git a/internal/rootcoord/alter_collection_task.go b/internal/rootcoord/alter_collection_task.go index dfedb86684..1af6583b71 100644 --- a/internal/rootcoord/alter_collection_task.go +++ b/internal/rootcoord/alter_collection_task.go @@ -171,11 +171,6 @@ func (a *alterCollectionFieldTask) Prepare(ctx context.Context) error { return fmt.Errorf("alter collection field failed, filed name does not exists") } - err := IsValidateUpdatedFieldProps(a.Req.GetProperties()) - if err != nil { - return err - } - return nil } @@ -229,29 +224,6 @@ func (a *alterCollectionFieldTask) Execute(ctx context.Context) error { return redoTask.Execute(ctx) } -var allowedProps = []string{ - common.MaxLengthKey, - common.MmapEnabledKey, -} - -func IsKeyAllowed(key string) bool { - for _, allowedKey := range allowedProps { - if key == allowedKey { - return true - } - } - return false -} - -func IsValidateUpdatedFieldProps(updatedProps []*commonpb.KeyValuePair) error { - for _, prop := range updatedProps { - if !IsKeyAllowed(prop.Key) { - return merr.WrapErrParameterInvalidMsg("%s does not allow update in collection field param", prop.Key) - } - } - return nil -} - func UpdateFieldProperties(coll *model.Collection, fieldName string, updatedProps []*commonpb.KeyValuePair) error { for i, field := range coll.Fields { if field.Name == fieldName { diff --git a/tests/python_client/requirements.txt b/tests/python_client/requirements.txt index e5c9ea2729..f29a2a6e73 100644 --- a/tests/python_client/requirements.txt +++ b/tests/python_client/requirements.txt @@ -27,8 +27,8 @@ pytest-parallel pytest-random-order # pymilvus -pymilvus==2.6.0rc12 -pymilvus[bulk_writer]==2.6.0rc12 +pymilvus==2.5.1rc9 +pymilvus[bulk_writer]==2.5.1rc9 # for customize config test