enhance: altercollection validation logic (#38419)

altercollection validation logic
issue: https://github.com/milvus-io/milvus/issues/37436

---------

Signed-off-by: Xianhui.Lin <xianhui.lin@zilliz.com>

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

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

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 {

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