From c8cab4dfc311934162ac06a665428c7839de7369 Mon Sep 17 00:00:00 2001 From: "cai.zhang" Date: Thu, 9 Nov 2023 22:10:32 +0800 Subject: [PATCH] No hit when the index exceeds the array length (#28302) Signed-off-by: Cai Zhang --- .../src/query/visitors/ExecExprVisitor.cpp | 66 +++ internal/core/unittest/test_array_expr.cpp | 444 +++++++++++++++++- tests/python_client/testcases/test_search.py | 20 +- 3 files changed, 518 insertions(+), 12 deletions(-) diff --git a/internal/core/src/query/visitors/ExecExprVisitor.cpp b/internal/core/src/query/visitors/ExecExprVisitor.cpp index 06bf573f05..2b7b87d95c 100644 --- a/internal/core/src/query/visitors/ExecExprVisitor.cpp +++ b/internal/core/src/query/visitors/ExecExprVisitor.cpp @@ -731,6 +731,9 @@ ExecExprVisitor::ExecUnaryRangeVisitorDispatcherArray(UnaryRangeExpr& expr_raw) if constexpr (std::is_same_v) { return array.is_same_array(val); } else { + if (index >= array.length()) { + return false; + } auto array_data = array.template get_data(index); return array_data == val; } @@ -743,6 +746,9 @@ ExecExprVisitor::ExecUnaryRangeVisitorDispatcherArray(UnaryRangeExpr& expr_raw) if constexpr (std::is_same_v) { return !array.is_same_array(val); } else { + if (index >= array.length()) { + return false; + } auto array_data = array.template get_data(index); return array_data != val; } @@ -755,6 +761,9 @@ ExecExprVisitor::ExecUnaryRangeVisitorDispatcherArray(UnaryRangeExpr& expr_raw) if constexpr (std::is_same_v) { return false; } else { + if (index >= array.length()) { + return false; + } auto array_data = array.template get_data(index); return array_data >= val; } @@ -767,6 +776,9 @@ ExecExprVisitor::ExecUnaryRangeVisitorDispatcherArray(UnaryRangeExpr& expr_raw) if constexpr (std::is_same_v) { return false; } else { + if (index >= array.length()) { + return false; + } auto array_data = array.template get_data(index); return array_data > val; } @@ -779,6 +791,9 @@ ExecExprVisitor::ExecUnaryRangeVisitorDispatcherArray(UnaryRangeExpr& expr_raw) if constexpr (std::is_same_v) { return false; } else { + if (index >= array.length()) { + return false; + } auto array_data = array.template get_data(index); return array_data <= val; } @@ -791,6 +806,9 @@ ExecExprVisitor::ExecUnaryRangeVisitorDispatcherArray(UnaryRangeExpr& expr_raw) if constexpr (std::is_same_v) { return false; } else { + if (index >= array.length()) { + return false; + } auto array_data = array.template get_data(index); return array_data < val; } @@ -803,6 +821,9 @@ ExecExprVisitor::ExecUnaryRangeVisitorDispatcherArray(UnaryRangeExpr& expr_raw) if constexpr (std::is_same_v) { return false; } else { + if (index >= array.length()) { + return false; + } auto array_data = array.template get_data(index); return Match(array_data, val, op); } @@ -1247,6 +1268,9 @@ ExecExprVisitor::ExecBinaryArithOpEvalRangeVisitorDispatcherArray( return false; }; auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return value + right_operand == val; }; @@ -1259,6 +1283,9 @@ ExecExprVisitor::ExecBinaryArithOpEvalRangeVisitorDispatcherArray( return false; }; auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return value - right_operand == val; }; @@ -1271,6 +1298,9 @@ ExecExprVisitor::ExecBinaryArithOpEvalRangeVisitorDispatcherArray( return false; }; auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return value * right_operand == val; }; @@ -1283,6 +1313,9 @@ ExecExprVisitor::ExecBinaryArithOpEvalRangeVisitorDispatcherArray( return false; }; auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return value / right_operand == val; }; @@ -1295,6 +1328,9 @@ ExecExprVisitor::ExecBinaryArithOpEvalRangeVisitorDispatcherArray( return false; }; auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return static_cast( fmod(value, right_operand)) == val; @@ -1328,6 +1364,9 @@ ExecExprVisitor::ExecBinaryArithOpEvalRangeVisitorDispatcherArray( return false; }; auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return value + right_operand != val; }; @@ -1340,6 +1379,9 @@ ExecExprVisitor::ExecBinaryArithOpEvalRangeVisitorDispatcherArray( return false; }; auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return value - right_operand != val; }; @@ -1352,6 +1394,9 @@ ExecExprVisitor::ExecBinaryArithOpEvalRangeVisitorDispatcherArray( return false; }; auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return value * right_operand != val; }; @@ -1364,6 +1409,9 @@ ExecExprVisitor::ExecBinaryArithOpEvalRangeVisitorDispatcherArray( return false; }; auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return value / right_operand != val; }; @@ -1376,6 +1424,9 @@ ExecExprVisitor::ExecBinaryArithOpEvalRangeVisitorDispatcherArray( return false; }; auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return static_cast( fmod(value, right_operand)) != val; @@ -1575,6 +1626,9 @@ ExecExprVisitor::ExecBinaryRangeVisitorDispatcherArray( if (lower_inclusive && upper_inclusive) { auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return val1 <= value && value <= val2; }; @@ -1582,6 +1636,9 @@ ExecExprVisitor::ExecBinaryRangeVisitorDispatcherArray( expr.column_.field_id, index_func, elem_func); } else if (lower_inclusive && !upper_inclusive) { auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return val1 <= value && value < val2; }; @@ -1589,6 +1646,9 @@ ExecExprVisitor::ExecBinaryRangeVisitorDispatcherArray( expr.column_.field_id, index_func, elem_func); } else if (!lower_inclusive && upper_inclusive) { auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return val1 < value && value <= val2; }; @@ -1596,6 +1656,9 @@ ExecExprVisitor::ExecBinaryRangeVisitorDispatcherArray( expr.column_.field_id, index_func, elem_func); } else { auto elem_func = [&](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return val1 < value && value < val2; }; @@ -2533,6 +2596,9 @@ ExecExprVisitor::ExecTermArrayFieldInVariable(TermExpr& expr_raw) } auto elem_func = [&term_set, &index](const milvus::ArrayView& array) { + if (index >= array.length()) { + return false; + } auto value = array.get_data(index); return term_set.find(ExprValueType(value)) != term_set.end(); }; diff --git a/internal/core/unittest/test_array_expr.cpp b/internal/core/unittest/test_array_expr.cpp index 0b03e1d180..89798a4417 100644 --- a/internal/core/unittest/test_array_expr.cpp +++ b/internal/core/unittest/test_array_expr.cpp @@ -57,6 +57,30 @@ TEST(Expr, TestArrayRange) { auto val = array.get_data(0); return 1 < val && val < 10000; }}, + {R"(binary_range_expr: < + column_info: < + field_id: 102 + data_type: Array + nested_path:"1024" + element_type:Int64 + > + lower_inclusive: false, + upper_inclusive: false, + lower_value: < + int64_val: 1 + > + upper_value: < + int64_val: 10000 + > + >)", + "long", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return 1 < val && val < 10000; + }}, {R"(binary_range_expr: < column_info: < field_id: 102 @@ -78,6 +102,30 @@ TEST(Expr, TestArrayRange) { auto val = array.get_data(0); return 1 <= val && val < 10000; }}, + {R"(binary_range_expr: < + column_info: < + field_id: 102 + data_type: Array + nested_path:"1024" + element_type:Int64 + > + lower_inclusive: true, + upper_inclusive: false, + lower_value: < + int64_val: 1 + > + upper_value: < + int64_val: 10000 + > + >)", + "long", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return 1 <= val && val < 10000; + }}, {R"(binary_range_expr: < column_info: < field_id: 102 @@ -99,6 +147,30 @@ TEST(Expr, TestArrayRange) { auto val = array.get_data(0); return 1 < val && val <= 10000; }}, + {R"(binary_range_expr: < + column_info: < + field_id: 102 + data_type: Array + nested_path:"1024" + element_type:Int64 + > + lower_inclusive: false, + upper_inclusive: true, + lower_value: < + int64_val: 1 + > + upper_value: < + int64_val: 10000 + > + >)", + "long", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return 1 < val && val <= 10000; + }}, {R"(binary_range_expr: < column_info: < field_id: 102 @@ -120,6 +192,30 @@ TEST(Expr, TestArrayRange) { auto val = array.get_data(0); return 1 <= val && val <= 10000; }}, + {R"(binary_range_expr: < + column_info: < + field_id: 102 + data_type: Array + nested_path:"1024" + element_type:Int64 + > + lower_inclusive: true, + upper_inclusive: true, + lower_value: < + int64_val: 1 + > + upper_value: < + int64_val: 10000 + > + >)", + "long", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return 1 <= val && val <= 10000; + }}, {R"(binary_range_expr: < column_info: < field_id: 104 @@ -315,6 +411,126 @@ TEST(Expr, TestArrayRange) { auto val = array.get_data(0); return val == 2.2; }}, + {R"(unary_range_expr: < + column_info: < + field_id: 105 + data_type: Array + nested_path:"1024" + element_type:Float + > + op: Equal, + value: < + float_val: 2.2 + > + >)", + "float", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val == 2.2; + }}, + {R"(unary_range_expr: < + column_info: < + field_id: 105 + data_type: Array + nested_path:"1024" + element_type:Float + > + op: NotEqual, + value: < + float_val: 2.2 + > + >)", + "float", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val != 2.2; + }}, + {R"(unary_range_expr: < + column_info: < + field_id: 105 + data_type: Array + nested_path:"1024" + element_type:Float + > + op: GreaterEqual, + value: < + float_val: 2.2 + > + >)", + "float", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val >= 2.2; + }}, + {R"(unary_range_expr: < + column_info: < + field_id: 105 + data_type: Array + nested_path:"1024" + element_type:Float + > + op: GreaterThan, + value: < + float_val: 2.2 + > + >)", + "float", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val > 2.2; + }}, + {R"(unary_range_expr: < + column_info: < + field_id: 105 + data_type: Array + nested_path:"1024" + element_type:Float + > + op: LessEqual, + value: < + float_val: 2.2 + > + >)", + "float", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val <= 2.2; + }}, + {R"(unary_range_expr: < + column_info: < + field_id: 105 + data_type: Array + nested_path:"1024" + element_type:Float + > + op: LessThan, + value: < + float_val: 2.2 + > + >)", + "float", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val < 2.2; + }}, }; std::string raw_plan_tmp = R"(vector_anns: < @@ -1224,6 +1440,206 @@ TEST(Expr, TestArrayBinaryArith) { auto val = array.get_data(0); return val % 3 != 2; }}, + {R"(binary_arith_op_eval_range_expr: < + column_info: < + field_id: 103 + data_type: Array + nested_path:"1024" + element_type:Float + > + arith_op:Add + right_operand: + op:Equal + value: + >)", + "float", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val + 2.2 == 133.2; + }}, + {R"(binary_arith_op_eval_range_expr: < + column_info: < + field_id: 103 + data_type: Array + nested_path:"1024" + element_type:Float + > + arith_op:Add + right_operand: + op:NotEqual + value: + >)", + "float", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val + 2.2 != 133.2; + }}, + {R"(binary_arith_op_eval_range_expr: < + column_info: < + field_id: 104 + data_type: Array + nested_path:"1024" + element_type:Double + > + arith_op:Sub + right_operand: + op:Equal + value: + >)", + "double", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val - 11.1 == 125.7; + }}, + {R"(binary_arith_op_eval_range_expr: < + column_info: < + field_id: 104 + data_type: Array + nested_path:"1024" + element_type:Double + > + arith_op:Sub + right_operand: + op:NotEqual + value: + >)", + "double", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val - 11.1 != 125.7; + }}, + {R"(binary_arith_op_eval_range_expr: < + column_info: < + field_id: 102 + data_type: Array + nested_path:"1024" + element_type:Int64 + > + arith_op:Mul + right_operand: + op:Equal + value: + >)", + "long", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val * 2 == 8; + }}, + {R"(binary_arith_op_eval_range_expr: < + column_info: < + field_id: 102 + data_type: Array + nested_path:"1024" + element_type:Int64 + > + arith_op:Mul + right_operand: + op:NotEqual + value: + >)", + "long", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val * 2 != 20; + }}, + {R"(binary_arith_op_eval_range_expr: < + column_info: < + field_id: 102 + data_type: Array + nested_path:"1024" + element_type:Int64 + > + arith_op:Div + right_operand: + op:Equal + value: + >)", + "long", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val / 2 == 8; + }}, + {R"(binary_arith_op_eval_range_expr: < + column_info: < + field_id: 102 + data_type: Array + nested_path:"1024" + element_type:Int64 + > + arith_op:Div + right_operand: + op:NotEqual + value: + >)", + "long", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val / 2 != 20; + }}, + {R"(binary_arith_op_eval_range_expr: < + column_info: < + field_id: 102 + data_type: Array + nested_path:"1024" + element_type:Int64 + > + arith_op:Mod + right_operand: + op:Equal + value: + >)", + "long", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val % 3 == 0; + }}, + {R"(binary_arith_op_eval_range_expr: < + column_info: < + field_id: 102 + data_type: Array + nested_path:"1024" + element_type:Int64 + > + arith_op:Mod + right_operand: + op:NotEqual + value: + >)", + "long", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val % 3 != 2; + }}, {R"(binary_arith_op_eval_range_expr: < column_info: < field_id: 101 @@ -1343,6 +1759,15 @@ TEST(Expr, TestArrayStringMatch) { [](milvus::Array& array) { return PrefixMatch(array.get_data(1), "def"); }}, + {OpType::PrefixMatch, + "def", + {"1024"}, + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + return PrefixMatch(array.get_data(1024), "def"); + }}, }; //vector_anns: op:PrefixMatch value: > > query_info:<> placeholder_tag:"$0" > for (auto& testcase : prefix_testcases) { @@ -1530,7 +1955,24 @@ TEST(Expr, TestArrayInTerm) { [](milvus::Array& array) { return false; }}, - }; + {R"(term_expr: < + column_info: < + field_id: 104 + data_type: Array + nested_path:"1024" + element_type:VarChar + > + values: values: + >)", + "string", + [](milvus::Array& array) { + if (array.length() <= 1024) { + return false; + } + auto val = array.get_data(1024); + return val == "abc" || val == "idhgf1s"; + }}, + }; std::string raw_plan_tmp = R"(vector_anns: < field_id: 100 diff --git a/tests/python_client/testcases/test_search.py b/tests/python_client/testcases/test_search.py index c157ff5f8b..f53f37d5a1 100644 --- a/tests/python_client/testcases/test_search.py +++ b/tests/python_client/testcases/test_search.py @@ -571,17 +571,15 @@ class TestCollectionSearchInvalid(TestcaseBase): # 2. search expression = "int32_array[101] > 0" - msg = ("failed to search: attempt #0: failed to search/query delegator 1 for channel " - "by-dev-rootcoord-dml_: fail to Search, QueryNode ID=1, reason=worker(1) query" - " failed: UnknownError: Assert \")index >= 0 && index < length_\" at /go/src/" - "github.com/milvus-io/milvus/internal/core/src/common/Array.h:454 => index out" - " of range, index=101, length=100: attempt #1: no available shard delegator " - "found: service unavailable") - collection_w.search(vectors[:default_nq], default_search_field, - default_search_params, nb, expression, - check_task=CheckTasks.err_res, - check_items={ct.err_code: 65538, - ct.err_msg: msg}) + # msg = ("failed to search: attempt #0: failed to search/query delegator 1 for channel " + # "by-dev-rootcoord-dml_: fail to Search, QueryNode ID=1, reason=worker(1) query" + # " failed: UnknownError: Assert \")index >= 0 && index < length_\" at /go/src/" + # "github.com/milvus-io/milvus/internal/core/src/common/Array.h:454 => index out" + # " of range, index=101, length=100: attempt #1: no available shard delegator " + # "found: service unavailable") + res, _ = collection_w.search(vectors[:default_nq], default_search_field, + default_search_params, nb, expression) + assert len(res[0]) == 0 @pytest.mark.tags(CaseLabel.L1) def test_search_with_expression_invalid_array_two(self):