From 8885fa2bc705558957c84ac9f3103f26e31adbcd Mon Sep 17 00:00:00 2001 From: Jiquan Long Date: Fri, 16 Jun 2023 17:10:39 +0800 Subject: [PATCH] Fix BinaryArithOpEvalRange integer overflow case (#24943) Signed-off-by: longjiquan --- internal/core/src/query/Parser.cpp | 27 ++++++++++++------- internal/core/src/query/PlanProto.cpp | 17 ++++-------- .../src/query/visitors/ExecExprVisitor.cpp | 18 +++++++------ .../src/query/visitors/ShowExprVisitor.cpp | 8 ++---- 4 files changed, 35 insertions(+), 35 deletions(-) diff --git a/internal/core/src/query/Parser.cpp b/internal/core/src/query/Parser.cpp index 0f06e3d191..af2336af32 100644 --- a/internal/core/src/query/Parser.cpp +++ b/internal/core/src/query/Parser.cpp @@ -306,21 +306,30 @@ Parser::ParseRangeNodeImpl(const FieldName& field_name, const Json& body) { } else if constexpr (std::is_integral_v) { Assert(right_operand.is_number_integer()); Assert(value.is_number_integer()); + // see also: https://github.com/milvus-io/milvus/issues/23646. + return std::make_unique< + BinaryArithOpEvalRangeExprImpl>( + ColumnInfo(schema.get_field_id(field_name), + schema[field_name].get_data_type()), + proto::plan::GenericValue::ValCase::kInt64Val, + arith_op_mapping_.at(arith_op_name), + right_operand, + mapping_.at(op_name), + value); } else if constexpr (std::is_floating_point_v) { Assert(right_operand.is_number()); Assert(value.is_number()); + return std::make_unique>( + ColumnInfo(schema.get_field_id(field_name), + schema[field_name].get_data_type()), + proto::plan::GenericValue::ValCase::kFloatVal, + arith_op_mapping_.at(arith_op_name), + right_operand, + mapping_.at(op_name), + value); } else { static_assert(always_false, "unsupported type"); } - - return std::make_unique>( - ColumnInfo(schema.get_field_id(field_name), - schema[field_name].get_data_type()), - proto::plan::GenericValue::ValCase::VAL_NOT_SET, - arith_op_mapping_.at(arith_op_name), - right_operand, - mapping_.at(op_name), - value); } if constexpr (std::is_same_v) { diff --git a/internal/core/src/query/PlanProto.cpp b/internal/core/src/query/PlanProto.cpp index b8bd6981e9..40a18b8b46 100644 --- a/internal/core/src/query/PlanProto.cpp +++ b/internal/core/src/query/PlanProto.cpp @@ -514,22 +514,15 @@ ProtoParser::ParseBinaryArithOpEvalRangeExpr( auto result = [&]() -> ExprPtr { switch (data_type) { - case DataType::INT8: { - return ExtractBinaryArithOpEvalRangeExprImpl( - field_id, data_type, expr_pb); - } - case DataType::INT16: { - return ExtractBinaryArithOpEvalRangeExprImpl( - field_id, data_type, expr_pb); - } - case DataType::INT32: { - return ExtractBinaryArithOpEvalRangeExprImpl( - field_id, data_type, expr_pb); - } + // see also: https://github.com/milvus-io/milvus/issues/23646. + case DataType::INT8: + case DataType::INT16: + case DataType::INT32: case DataType::INT64: { return ExtractBinaryArithOpEvalRangeExprImpl( field_id, data_type, expr_pb); } + case DataType::FLOAT: { return ExtractBinaryArithOpEvalRangeExprImpl( field_id, data_type, expr_pb); diff --git a/internal/core/src/query/visitors/ExecExprVisitor.cpp b/internal/core/src/query/visitors/ExecExprVisitor.cpp index 62362c3c9b..b02009fe6b 100644 --- a/internal/core/src/query/visitors/ExecExprVisitor.cpp +++ b/internal/core/src/query/visitors/ExecExprVisitor.cpp @@ -585,13 +585,18 @@ template auto ExecExprVisitor::ExecBinaryArithOpEvalRangeVisitorDispatcher( BinaryArithOpEvalRangeExpr& expr_raw) -> BitsetType { - auto& expr = static_cast&>(expr_raw); + // see also: https://github.com/milvus-io/milvus/issues/23646. + typedef std::conditional_t, int64_t, T> + HighPrecisionType; + + auto& expr = + static_cast&>( + expr_raw); using Index = index::ScalarIndex; auto arith_op = expr.arith_op_; auto right_operand = expr.right_operand_; auto op = expr.op_type_; auto val = expr.value_; - auto& nested_path = expr.column_.nested_path; switch (op) { case OpType::Equal: { @@ -602,12 +607,9 @@ ExecExprVisitor::ExecBinaryArithOpEvalRangeVisitorDispatcher( auto x = index->Reverse_Lookup(offset); return (x + right_operand) == val; }; - auto elem_func = - [val, right_operand, &nested_path](MayConstRef x) { - // visit the nested field - // now it must be Json - return ((x + right_operand) == val); - }; + auto elem_func = [val, right_operand](MayConstRef x) { + return ((x + right_operand) == val); + }; return ExecDataRangeVisitorImpl( expr.column_.field_id, index_func, elem_func); } diff --git a/internal/core/src/query/visitors/ShowExprVisitor.cpp b/internal/core/src/query/visitors/ShowExprVisitor.cpp index 5d75770e4a..25e67f8b48 100644 --- a/internal/core/src/query/visitors/ShowExprVisitor.cpp +++ b/internal/core/src/query/visitors/ShowExprVisitor.cpp @@ -299,18 +299,14 @@ ShowExprVisitor::visit(BinaryArithOpEvalRangeExpr& expr) { AssertInfo(datatype_is_vector(expr.column_.data_type) == false, "[ShowExprVisitor]Data type of expr isn't vector type"); switch (expr.column_.data_type) { + // see also: https://github.com/milvus-io/milvus/issues/23646. case DataType::INT8: - json_opt_ = BinaryArithOpEvalRangeExtract(expr); - return; case DataType::INT16: - json_opt_ = BinaryArithOpEvalRangeExtract(expr); - return; case DataType::INT32: - json_opt_ = BinaryArithOpEvalRangeExtract(expr); - return; case DataType::INT64: json_opt_ = BinaryArithOpEvalRangeExtract(expr); return; + case DataType::DOUBLE: json_opt_ = BinaryArithOpEvalRangeExtract(expr); return;