Skip to content

Commit

Permalink
fix: Check if the dynamic fields contain any static fields (#38025)
Browse files Browse the repository at this point in the history
issue: #38024

Signed-off-by: Cai Zhang <[email protected]>
  • Loading branch information
xiaocai2333 authored Dec 1, 2024
1 parent ec119f5 commit 2319018
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 40 deletions.
12 changes: 11 additions & 1 deletion internal/proxy/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -1792,11 +1792,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())
}
}
}
}
}
Expand Down
29 changes: 29 additions & 0 deletions internal/proxy/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2269,6 +2269,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{}{
Expand Down
15 changes: 0 additions & 15 deletions internal/proxy/validate_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"go.uber.org/zap"

"github.com/milvus-io/milvus-proto/go-api/v2/schemapb"
"github.com/milvus-io/milvus/internal/json"
"github.com/milvus-io/milvus/internal/util/nullutil"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/log"
Expand Down Expand Up @@ -658,20 +657,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
}

Expand Down
24 changes: 0 additions & 24 deletions internal/proxy/validate_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6382,30 +6382,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) {
Expand Down

0 comments on commit 2319018

Please sign in to comment.