fix: not return err if consistencyLevel is not set to a valid value (#36714)

https://github.com/milvus-io/milvus/issues/36444 Signed-off-by: lixinguo <xinguo.li@zilliz.com> Co-authored-by: lixinguo <xinguo.li@zilliz.com>
2024-10-16 13:23:22 +08:00 · 2024-10-16 13:23:22 +08:00 · 44d80c1355
parent 72dc07ba48
commit 44d80c1355
6 changed files with 52 additions and 17 deletions
--- a/internal/distributed/proxy/httpserver/handler_v2.go
+++ b/internal/distributed/proxy/httpserver/handler_v2.go
@ -923,14 +923,14 @@ func generatePlaceholderGroup(ctx context.Context, body string, collSchema *sche
 	})
 }

-func generateSearchParams(ctx context.Context, c *gin.Context, reqSearchParams searchParams) ([]*commonpb.KeyValuePair, error) {
+func generateSearchParams(ctx context.Context, c *gin.Context, reqSearchParams searchParams) []*commonpb.KeyValuePair {
 	var searchParams []*commonpb.KeyValuePair
 	bs, _ := json.Marshal(reqSearchParams.Params)
 	searchParams = append(searchParams, &commonpb.KeyValuePair{Key: Params, Value: string(bs)})
 	searchParams = append(searchParams, &commonpb.KeyValuePair{Key: common.IgnoreGrowing, Value: strconv.FormatBool(reqSearchParams.IgnoreGrowing)})
 	// need to exposure ParamRoundDecimal in req?
 	searchParams = append(searchParams, &commonpb.KeyValuePair{Key: ParamRoundDecimal, Value: "-1"})
-	return searchParams, nil
+	return searchParams
 }

 func (h *HandlersV2) search(ctx context.Context, c *gin.Context, anyReq any, dbName string) (interface{}, error) {
@ -946,19 +946,22 @@ func (h *HandlersV2) search(ctx context.Context, c *gin.Context, anyReq any, dbN
 	var err error
 	req.ConsistencyLevel, req.UseDefaultConsistency, err = convertConsistencyLevel(httpReq.ConsistencyLevel)
 	if err != nil {
+		log.Ctx(ctx).Warn("high level restful api, search with consistency_level invalid", zap.Error(err))
+		HTTPAbortReturn(c, http.StatusOK, gin.H{
+			HTTPReturnCode:    merr.Code(err),
+			HTTPReturnMessage: "consistencyLevel can only be [Strong, Session, Bounded, Eventually, Customized], default: Bounded, err:" + err.Error(),
+		})
 		return nil, err
 	}
 	c.Set(ContextRequest, req)

 	collSchema, err := h.GetCollectionSchema(ctx, c, dbName, httpReq.CollectionName)
 	if err != nil {
+		// has already throw http in GetCollectionSchema if fails to get schema
 		return nil, err
 	}

-	searchParams, err := generateSearchParams(ctx, c, httpReq.SearchParams)
-	if err != nil {
-		return nil, err
-	}
+	searchParams := generateSearchParams(ctx, c, httpReq.SearchParams)
 	searchParams = append(searchParams, &commonpb.KeyValuePair{Key: common.TopKKey, Value: strconv.FormatInt(int64(httpReq.Limit), 10)})
 	searchParams = append(searchParams, &commonpb.KeyValuePair{Key: ParamOffset, Value: strconv.FormatInt(int64(httpReq.Offset), 10)})
 	searchParams = append(searchParams, &commonpb.KeyValuePair{Key: ParamGroupByField, Value: httpReq.GroupByField})
@ -1013,21 +1016,24 @@ func (h *HandlersV2) advancedSearch(ctx context.Context, c *gin.Context, anyReq
 	var err error
 	req.ConsistencyLevel, req.UseDefaultConsistency, err = convertConsistencyLevel(httpReq.ConsistencyLevel)
 	if err != nil {
+		log.Ctx(ctx).Warn("high level restful api, search with consistency_level invalid", zap.Error(err))
+		HTTPAbortReturn(c, http.StatusOK, gin.H{
+			HTTPReturnCode:    merr.Code(err),
+			HTTPReturnMessage: "consistencyLevel can only be [Strong, Session, Bounded, Eventually, Customized], default: Bounded, err:" + err.Error(),
+		})
 		return nil, err
 	}
 	c.Set(ContextRequest, req)

 	collSchema, err := h.GetCollectionSchema(ctx, c, dbName, httpReq.CollectionName)
 	if err != nil {
+		// has already throw http in GetCollectionSchema if fails to get schema
 		return nil, err
 	}
 	body, _ := c.Get(gin.BodyBytesKey)
 	searchArray := gjson.Get(string(body.([]byte)), "search").Array()
 	for i, subReq := range httpReq.Search {
-		searchParams, err := generateSearchParams(ctx, c, subReq.SearchParams)
-		if err != nil {
-			return nil, err
-		}
+		searchParams := generateSearchParams(ctx, c, subReq.SearchParams)
 		searchParams = append(searchParams, &commonpb.KeyValuePair{Key: common.TopKKey, Value: strconv.FormatInt(int64(subReq.Limit), 10)})
 		searchParams = append(searchParams, &commonpb.KeyValuePair{Key: ParamOffset, Value: strconv.FormatInt(int64(subReq.Offset), 10)})
 		searchParams = append(searchParams, &commonpb.KeyValuePair{Key: proxy.AnnsFieldKey, Value: subReq.AnnsField})
--- a/internal/distributed/proxy/httpserver/handler_v2_test.go
+++ b/internal/distributed/proxy/httpserver/handler_v2_test.go
@ -1732,6 +1732,12 @@ func TestSearchV2(t *testing.T) {
 		Status:         &StatusSuccess,
 	}, nil).Times(10)
 	mp.EXPECT().Search(mock.Anything, mock.Anything).Return(&milvuspb.SearchResults{Status: commonSuccessStatus, Results: &schemapb.SearchResultData{TopK: int64(0)}}, nil).Times(3)
+	mp.EXPECT().DescribeCollection(mock.Anything, mock.Anything).Return(&milvuspb.DescribeCollectionResponse{
+		Status: &commonpb.Status{
+			Code:   1100,
+			Reason: "mock",
+		},
+	}, nil).Once()
 	testEngine := initHTTPServerV2(mp, false)
 	queryTestCases := []requestBodyTestCase{}
 	queryTestCases = append(queryTestCases, requestBodyTestCase{
@ -1748,8 +1754,8 @@ func TestSearchV2(t *testing.T) {
 	})
 	queryTestCases = append(queryTestCases, requestBodyTestCase{
 		path:        SearchAction,
-		requestBody: []byte(`{"collectionName": "book", "data": [[0.1, 0.2]], "filter": "book_id in [2, 4, 6, 8]", "limit": 4, "outputFields": ["word_count"], "searchParams": {"ignore_growing": "true"}}`),
-		errMsg:      "can only accept json format request, error: json: cannot unmarshal string into Go struct field searchParams.searchParams.ignore_growing of type bool",
+		requestBody: []byte(`{"collectionName": "book", "data": [[0.1, 0.2]], "filter": "book_id in [2, 4, 6, 8]", "limit": 4, "outputFields": ["word_count"], "searchParams": {"ignoreGrowing": "true"}}`),
+		errMsg:      "can only accept json format request, error: json: cannot unmarshal string into Go struct field searchParams.searchParams.ignoreGrowing of type bool",
 		errCode:     1801, // ErrIncorrectParameterFormat
 	})
 	queryTestCases = append(queryTestCases, requestBodyTestCase{
@ -1823,6 +1829,17 @@ func TestSearchV2(t *testing.T) {
 			`{"data": ["AQIDBA=="], "annsField": "bfloat16Vector", "metricType": "L2", "limit": 3}` +
 			`], "rerank": {"strategy": "weighted", "params": {"weights":  [0.9, 0.8]}}}`),
 	})
+	queryTestCases = append(queryTestCases, requestBodyTestCase{
+		path: AdvancedSearchAction,
+		requestBody: []byte(`{"collectionName": "hello_milvus", "search": [` +
+			`{"data": [[0.1, 0.2]], "annsField": "book_intro", "metricType": "L2", "limit": 3},` +
+			`{"data": ["AQ=="], "annsField": "binaryVector", "metricType": "L2", "limit": 3},` +
+			`{"data": ["AQIDBA=="], "annsField": "float16Vector", "metricType": "L2", "limit": 3},` +
+			`{"data": ["AQIDBA=="], "annsField": "bfloat16Vector", "metricType": "L2", "limit": 3}` +
+			`], "consistencyLevel":"unknown","rerank": {"strategy": "weighted", "params": {"weights":  [0.9, 0.8]}}}`),
+		errMsg:  "consistencyLevel can only be [Strong, Session, Bounded, Eventually, Customized], default: Bounded, err:parameter:'unknown' is incorrect, please check it: invalid parameter",
+		errCode: 1100, // ErrParameterInvalid
+	})
 	queryTestCases = append(queryTestCases, requestBodyTestCase{
 		path: AdvancedSearchAction,
 		requestBody: []byte(`{"collectionName": "hello_milvus", "search": [` +
@ -1877,6 +1894,18 @@ func TestSearchV2(t *testing.T) {
 		errMsg:      "can only accept json format request, error: json: cannot unmarshal string into Go struct field searchParams.searchParams.params of type map[string]interface {}",
 		errCode:     1801, // ErrIncorrectParameterFormat
 	})
+	queryTestCases = append(queryTestCases, requestBodyTestCase{
+		path:        SearchAction,
+		requestBody: []byte(`{"collectionName": "book", "data": [[0.1, 0.2]], "filter": "book_id in [2, 4, 6, 8]", "limit": 4, "outputFields": ["word_count"],"consistencyLevel": "unknown"}`),
+		errMsg:      "consistencyLevel can only be [Strong, Session, Bounded, Eventually, Customized], default: Bounded, err:parameter:'unknown' is incorrect, please check it: invalid parameter",
+		errCode:     1100, // ErrParameterInvalid
+	})
+	queryTestCases = append(queryTestCases, requestBodyTestCase{
+		path:        SearchAction,
+		requestBody: []byte(`{"collectionName": "book", "data": ["AQ=="], "annsField": "binaryVector", "filter": "book_id in [2, 4, 6, 8]", "limit": 4, "outputFields": ["word_count"]}`),
+		errMsg:      "mock",
+		errCode:     1100, // ErrParameterInvalid
+	})

 	for _, testcase := range queryTestCases {
 		t.Run(testcase.path, func(t *testing.T) {
--- a/internal/distributed/proxy/httpserver/request_v2.go
+++ b/internal/distributed/proxy/httpserver/request_v2.go
@ -145,7 +145,7 @@ type searchParams struct {
 	// not use metricType any more, just for compatibility
 	MetricType    string                 `json:"metricType"`
 	Params        map[string]interface{} `json:"params"`
-	IgnoreGrowing bool                   `json:"ignore_growing"`
+	IgnoreGrowing bool                   `json:"ignoreGrowing"`
 }

 type SearchReqV2 struct {
--- a/internal/distributed/proxy/httpserver/utils.go
+++ b/internal/distributed/proxy/httpserver/utils.go
@ -1397,8 +1397,8 @@ func convertConsistencyLevel(reqConsistencyLevel string) (commonpb.ConsistencyLe
 		}
 		return commonpb.ConsistencyLevel(level), false, nil
 	}
-	// ConsistencyLevel_Session default in PyMilvus
-	return commonpb.ConsistencyLevel_Session, true, nil
+	// ConsistencyLevel_Bounded default in PyMilvus
+	return commonpb.ConsistencyLevel_Bounded, true, nil
 }

 func convertDefaultValue(value interface{}, dataType schemapb.DataType) (*schemapb.ValueField, error) {
--- a/internal/distributed/proxy/httpserver/utils_test.go
+++ b/internal/distributed/proxy/httpserver/utils_test.go
@ -1728,7 +1728,7 @@ func TestBuildQueryResps(t *testing.T) {
 func TestConvertConsistencyLevel(t *testing.T) {
 	consistencyLevel, useDefaultConsistency, err := convertConsistencyLevel("")
 	assert.Equal(t, nil, err)
-	assert.Equal(t, consistencyLevel, commonpb.ConsistencyLevel_Session)
+	assert.Equal(t, consistencyLevel, commonpb.ConsistencyLevel_Bounded)
 	assert.Equal(t, true, useDefaultConsistency)
 	consistencyLevel, useDefaultConsistency, err = convertConsistencyLevel("Strong")
 	assert.Equal(t, nil, err)
--- a/tests/restful_client_v2/testcases/test_vector_operations.py
+++ b/tests/restful_client_v2/testcases/test_vector_operations.py
@ -1651,7 +1651,7 @@ class TestSearchVector(TestBase):
            "limit": limit,
            "offset": 0,
            "searchParams": {
-                "ignore_growing": ignore_growing
+                "ignoreGrowing": ignore_growing

            }
        }