From 580caebb31c98e4dcb31a8663918b66fc71f4158 Mon Sep 17 00:00:00 2001 From: "cai.zhang" Date: Mon, 2 Dec 2024 12:22:38 +0800 Subject: [PATCH] fix: [2.4]Check if the dynamic fields contain any static fields (#38027) issue: #38024 master pr: #38025 Signed-off-by: Cai Zhang --- internal/proxy/util.go | 12 +++++++++++- internal/proxy/util_test.go | 29 ++++++++++++++++++++++++++++ internal/proxy/validate_util.go | 15 -------------- internal/proxy/validate_util_test.go | 24 ----------------------- 4 files changed, 40 insertions(+), 40 deletions(-) diff --git a/internal/proxy/util.go b/internal/proxy/util.go index 55c4befe7b517..3817a643d3c1a 100644 --- 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 index 0aed56b3a4b09..acf97c1981619 100644 --- 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 index f01ca2a8a1952..4b45605a61713 100644 --- 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 index 353bba6852412..01f6f29c8582f 100644 --- 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) {