From c88f1be985512e5fcc8136bccf830cb540f52b0e Mon Sep 17 00:00:00 2001 From: zhagnlu <1542303831@qq.com> Date: Mon, 23 Jun 2025 20:36:46 +0800 Subject: [PATCH] fix:fix wrong use return error for parse unsupported arith (#42729) (#42890) pr: #42729 Signed-off-by: luzhang Co-authored-by: luzhang --- internal/parser/planparserv2/operators.go | 30 +++++++++---------- .../parser/planparserv2/parser_visitor.go | 24 ++++++++++++--- .../planparserv2/plan_parser_v2_test.go | 17 +++++++++++ 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/internal/parser/planparserv2/operators.go b/internal/parser/planparserv2/operators.go index a6f4e2476f..09c0abf0b5 100644 --- a/internal/parser/planparserv2/operators.go +++ b/internal/parser/planparserv2/operators.go @@ -62,7 +62,7 @@ var binaryLogicalNameMap = map[int]string{ parser.PlanParserOR: "or", } -func Add(a, b *planpb.GenericValue) *ExprWithType { +func Add(a, b *planpb.GenericValue) (*ExprWithType, error) { ret := &ExprWithType{ expr: &planpb.Expr{ Expr: &planpb.Expr_ValueExpr{ @@ -72,11 +72,11 @@ func Add(a, b *planpb.GenericValue) *ExprWithType { } if IsBool(a) || IsBool(b) { - return nil + return nil, errors.New("add cannot apply on bool field") } if IsString(a) || IsString(b) { - return nil + return nil, errors.New("add cannot apply on string field") } aFloat, bFloat, aInt, bInt := IsFloating(a), IsFloating(b), IsInteger(a), IsInteger(b) @@ -96,10 +96,10 @@ func Add(a, b *planpb.GenericValue) *ExprWithType { ret.expr.GetValueExpr().Value = NewInt(a.GetInt64Val() + b.GetInt64Val()) } - return ret + return ret, nil } -func Subtract(a, b *planpb.GenericValue) *ExprWithType { +func Subtract(a, b *planpb.GenericValue) (*ExprWithType, error) { ret := &ExprWithType{ expr: &planpb.Expr{ Expr: &planpb.Expr_ValueExpr{ @@ -109,11 +109,11 @@ func Subtract(a, b *planpb.GenericValue) *ExprWithType { } if IsBool(a) || IsBool(b) { - return nil + return nil, errors.New("subtract cannot apply on bool field") } if IsString(a) || IsString(b) { - return nil + return nil, errors.New("subtract cannot apply on string field") } aFloat, bFloat, aInt, bInt := IsFloating(a), IsFloating(b), IsInteger(a), IsInteger(b) @@ -133,10 +133,10 @@ func Subtract(a, b *planpb.GenericValue) *ExprWithType { ret.expr.GetValueExpr().Value = NewInt(a.GetInt64Val() - b.GetInt64Val()) } - return ret + return ret, nil } -func Multiply(a, b *planpb.GenericValue) *ExprWithType { +func Multiply(a, b *planpb.GenericValue) (*ExprWithType, error) { ret := &ExprWithType{ expr: &planpb.Expr{ Expr: &planpb.Expr_ValueExpr{ @@ -146,11 +146,11 @@ func Multiply(a, b *planpb.GenericValue) *ExprWithType { } if IsBool(a) || IsBool(b) { - return nil + return nil, errors.New("multiply cannot apply on bool field") } if IsString(a) || IsString(b) { - return nil + return nil, errors.New("multiply cannot apply on string field") } aFloat, bFloat, aInt, bInt := IsFloating(a), IsFloating(b), IsInteger(a), IsInteger(b) @@ -170,7 +170,7 @@ func Multiply(a, b *planpb.GenericValue) *ExprWithType { ret.expr.GetValueExpr().Value = NewInt(a.GetInt64Val() * b.GetInt64Val()) } - return ret + return ret, nil } func Divide(a, b *planpb.GenericValue) (*ExprWithType, error) { @@ -373,9 +373,9 @@ func Negative(a *planpb.GenericValue) *ExprWithType { return nil } -func Not(a *planpb.GenericValue) *ExprWithType { +func Not(a *planpb.GenericValue) (*ExprWithType, error) { if !IsBool(a) { - return nil + return nil, errors.New("not can only apply on boolean") } return &ExprWithType{ dataType: schemapb.DataType_Bool, @@ -386,7 +386,7 @@ func Not(a *planpb.GenericValue) *ExprWithType { }, }, }, - } + }, nil } /* diff --git a/internal/parser/planparserv2/parser_visitor.go b/internal/parser/planparserv2/parser_visitor.go index 1524f01299..65a8242e56 100644 --- a/internal/parser/planparserv2/parser_visitor.go +++ b/internal/parser/planparserv2/parser_visitor.go @@ -179,9 +179,17 @@ func (v *ParserVisitor) VisitAddSub(ctx *parser.AddSubContext) interface{} { leftValue, rightValue := leftValueExpr.GetValue(), rightValueExpr.GetValue() switch ctx.GetOp().GetTokenType() { case parser.PlanParserADD: - return Add(leftValue, rightValue) + n, err := Add(leftValue, rightValue) + if err != nil { + return err + } + return n case parser.PlanParserSUB: - return Subtract(leftValue, rightValue) + n, err := Subtract(leftValue, rightValue) + if err != nil { + return err + } + return n default: return fmt.Errorf("unexpected op: %s", ctx.GetOp().GetText()) } @@ -258,7 +266,11 @@ func (v *ParserVisitor) VisitMulDivMod(ctx *parser.MulDivModContext) interface{} leftValue, rightValue := getGenericValue(left), getGenericValue(right) switch ctx.GetOp().GetTokenType() { case parser.PlanParserMUL: - return Multiply(leftValue, rightValue) + n, err := Multiply(leftValue, rightValue) + if err != nil { + return err + } + return n case parser.PlanParserDIV: n, err := Divide(leftValue, rightValue) if err != nil { @@ -804,7 +816,11 @@ func (v *ParserVisitor) VisitUnary(ctx *parser.UnaryContext) interface{} { case parser.PlanParserSUB: return Negative(childValue) case parser.PlanParserNOT: - return Not(childValue) + n, err := Not(childValue) + if err != nil { + return err + } + return n default: return fmt.Errorf("unexpected op: %s", ctx.GetOp().GetText()) } diff --git a/internal/parser/planparserv2/plan_parser_v2_test.go b/internal/parser/planparserv2/plan_parser_v2_test.go index fd6f292790..398563e7e5 100644 --- a/internal/parser/planparserv2/plan_parser_v2_test.go +++ b/internal/parser/planparserv2/plan_parser_v2_test.go @@ -163,6 +163,23 @@ func TestExpr_Compare(t *testing.T) { for _, exprStr := range exprStrs { assertValidExpr(t, helper, exprStr) } + + exprStrs = []string{ + `BoolField == false + true`, + `StringField == "1" + "2"`, + `BoolField == false - true`, + `StringField == "1" - "2"`, + `BoolField == false * true`, + `StringField == "1" * "2"`, + `BoolField == false / true`, + `StringField == "1" / "2"`, + `BoolField == false % true`, + `StringField == "1" % "2"`, + } + + for _, exprStr := range exprStrs { + assertInvalidExpr(t, helper, exprStr) + } } func TestExpr_UnaryRange(t *testing.T) {