fix: [2.4]Check if the dynamic fields contain any static fields (milv…

…us-io#38027) issue: milvus-io#38024 master pr: milvus-io#38025 Signed-off-by: Cai Zhang <[email protected]>
shaoting-huang · Dec 2, 2024 · 580caeb · 580caeb
1 parent c32ad65
commit 580caeb
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 40 deletions.
diff --git a/internal/proxy/util.go b/internal/proxy/util.go
@@ -1614,11 +1614,21 @@ func verifyDynamicFieldData(schema *schemapb.CollectionSchema, insertMsg *msgstr
 			for _, rowData := range field.GetScalars().GetJsonData().GetData() {
 				jsonData := make(map[string]interface{})
 				if err := json.Unmarshal(rowData, &jsonData); err != nil {
-					return err
+					log.Info("insert invalid dynamic data, milvus only support json map",
+						zap.ByteString("data", rowData),
+						zap.Error(err),
+					)
+					return merr.WrapErrIoFailedReason(err.Error())
 				}
 				if _, ok := jsonData[common.MetaFieldName]; ok {
 					return fmt.Errorf("cannot set json key to: %s", common.MetaFieldName)
 				}
+				for _, f := range schema.GetFields() {
+					if _, ok := jsonData[f.GetName()]; ok {
+						log.Info("dynamic field name include the static field name", zap.String("fieldName", f.GetName()))
+						return fmt.Errorf("dynamic field name cannot include the static field name: %s", f.GetName())
+					}
+				}
 			}
 		}
 	}

diff --git a/internal/proxy/util_test.go b/internal/proxy/util_test.go
@@ -2258,6 +2258,35 @@ func Test_CheckDynamicFieldData(t *testing.T) {
 		err = checkDynamicFieldData(schema, insertMsg)
 		assert.Error(t, err)
 	})
+	t.Run("key has static field name", func(t *testing.T) {
+		jsonData := make([][]byte, 0)
+		data := map[string]interface{}{
+			"bool":   true,
+			"int":    100,
+			"float":  1.2,
+			"string": "abc",
+			"json": map[string]interface{}{
+				"int":   20,
+				"array": []int{1, 2, 3},
+			},
+			"Int64Field": "error key",
+		}
+		jsonBytes, err := json.MarshalIndent(data, "", "  ")
+		assert.NoError(t, err)
+		jsonData = append(jsonData, jsonBytes)
+		jsonFieldData := autoGenDynamicFieldData(jsonData)
+		schema := newTestSchema()
+		insertMsg := &msgstream.InsertMsg{
+			InsertRequest: &msgpb.InsertRequest{
+				CollectionName: "collectionName",
+				FieldsData:     []*schemapb.FieldData{jsonFieldData},
+				NumRows:        1,
+				Version:        msgpb.InsertDataVersion_ColumnBased,
+			},
+		}
+		err = checkDynamicFieldData(schema, insertMsg)
+		assert.Error(t, err)
+	})
 	t.Run("disable dynamic schema", func(t *testing.T) {
 		jsonData := make([][]byte, 0)
 		data := map[string]interface{}{

diff --git a/internal/proxy/validate_util.go b/internal/proxy/validate_util.go
@@ -1,7 +1,6 @@
 package proxy
 
 import (
-	"encoding/json"
 	"fmt"
 	"math"
 	"reflect"
@@ -446,20 +445,6 @@ func (v *validateUtil) checkJSONFieldData(field *schemapb.FieldData, fieldSchema
 			}
 		}
 	}
-
-	if fieldSchema.GetIsDynamic() {
-		var jsonMap map[string]interface{}
-		for _, data := range jsonArray {
-			err := json.Unmarshal(data, &jsonMap)
-			if err != nil {
-				log.Warn("insert invalid JSON data, milvus only support json map without nesting",
-					zap.ByteString("data", data),
-					zap.Error(err),
-				)
-				return merr.WrapErrIoFailedReason(err.Error())
-			}
-		}
-	}
 	return nil
 }
 

diff --git a/internal/proxy/validate_util_test.go b/internal/proxy/validate_util_test.go
@@ -3902,30 +3902,6 @@ func Test_validateUtil_checkJSONData(t *testing.T) {
 		err := v.checkJSONFieldData(data, f)
 		assert.Error(t, err)
 	})
-
-	t.Run("invalid_JSON_data", func(t *testing.T) {
-		v := newValidateUtil(withOverflowCheck(), withMaxLenCheck())
-		jsonData := "hello"
-		f := &schemapb.FieldSchema{
-			DataType:  schemapb.DataType_JSON,
-			IsDynamic: true,
-		}
-		data := &schemapb.FieldData{
-			FieldName: "json",
-			Field: &schemapb.FieldData_Scalars{
-				Scalars: &schemapb.ScalarField{
-					Data: &schemapb.ScalarField_JsonData{
-						JsonData: &schemapb.JSONArray{
-							Data: [][]byte{[]byte(jsonData)},
-						},
-					},
-				},
-			},
-		}
-
-		err := v.checkJSONFieldData(data, f)
-		assert.Error(t, err)
-	})
 }
 
 func Test_validateUtil_checkLongFieldData(t *testing.T) {