mirror of https://github.com/milvus-io/milvus.git
Fix expression incompatible between parser and executor (#26493)
Signed-off-by: longjiquan <jiquan.long@zilliz.com>pull/26523/head
parent
85886f039e
commit
2a517d2da1
|
@ -8,6 +8,10 @@ import (
|
|||
type ExprWithType struct {
|
||||
expr *planpb.Expr
|
||||
dataType schemapb.DataType
|
||||
// ExprWithType can be a node only when nodeDependent is set to false.
|
||||
// For example, a column expression or a value expression itself cannot be an expression node independently.
|
||||
// Unless our execution backend can support them.
|
||||
nodeDependent bool
|
||||
}
|
||||
|
||||
func getError(obj interface{}) error {
|
||||
|
|
|
@ -53,7 +53,8 @@ func (v *ParserVisitor) translateIdentifier(identifier string) (*ExprWithType, e
|
|||
},
|
||||
},
|
||||
},
|
||||
dataType: field.DataType,
|
||||
dataType: field.DataType,
|
||||
nodeDependent: true,
|
||||
}, nil
|
||||
}
|
||||
|
||||
|
@ -83,6 +84,7 @@ func (v *ParserVisitor) VisitBoolean(ctx *parser.BooleanContext) interface{} {
|
|||
},
|
||||
},
|
||||
},
|
||||
nodeDependent: true,
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -102,6 +104,7 @@ func (v *ParserVisitor) VisitInteger(ctx *parser.IntegerContext) interface{} {
|
|||
},
|
||||
},
|
||||
},
|
||||
nodeDependent: true,
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -121,6 +124,7 @@ func (v *ParserVisitor) VisitFloating(ctx *parser.FloatingContext) interface{} {
|
|||
},
|
||||
},
|
||||
},
|
||||
nodeDependent: true,
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -143,6 +147,7 @@ func (v *ParserVisitor) VisitString(ctx *parser.StringContext) interface{} {
|
|||
},
|
||||
},
|
||||
},
|
||||
nodeDependent: true,
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -213,8 +218,9 @@ func (v *ParserVisitor) VisitAddSub(ctx *parser.AddSubContext) interface{} {
|
|||
return err
|
||||
}
|
||||
return &ExprWithType{
|
||||
expr: expr,
|
||||
dataType: dataType,
|
||||
expr: expr,
|
||||
dataType: dataType,
|
||||
nodeDependent: true,
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -304,8 +310,9 @@ func (v *ParserVisitor) VisitMulDivMod(ctx *parser.MulDivModContext) interface{}
|
|||
return err
|
||||
}
|
||||
return &ExprWithType{
|
||||
expr: expr,
|
||||
dataType: dataType,
|
||||
expr: expr,
|
||||
dataType: dataType,
|
||||
nodeDependent: true,
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1019,7 +1026,8 @@ func (v *ParserVisitor) VisitJSONIdentifier(ctx *parser.JSONIdentifierContext) i
|
|||
},
|
||||
},
|
||||
},
|
||||
dataType: jsonField.GetDataType(),
|
||||
dataType: jsonField.GetDataType(),
|
||||
nodeDependent: true,
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1134,6 +1142,7 @@ func (v *ParserVisitor) VisitArray(ctx *parser.ArrayContext) interface{} {
|
|||
},
|
||||
},
|
||||
},
|
||||
nodeDependent: true,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -62,7 +62,7 @@ func ParseExpr(schema *typeutil.SchemaHelper, exprStr string) (*planpb.Expr, err
|
|||
if predicate == nil {
|
||||
return nil, fmt.Errorf("cannot parse expression: %s", exprStr)
|
||||
}
|
||||
if !typeutil.IsBoolType(predicate.dataType) {
|
||||
if !canBeExecuted(predicate) {
|
||||
return nil, fmt.Errorf("predicate is not a boolean expression: %s, data type: %s", exprStr, predicate.dataType)
|
||||
}
|
||||
|
||||
|
|
|
@ -474,6 +474,10 @@ func TestExpr_Invalid(t *testing.T) {
|
|||
`1 ** 2`,
|
||||
`1 << 2`,
|
||||
`1 | 2`,
|
||||
// -------------------- cannot be independent ----------------------
|
||||
`BoolField`,
|
||||
`true`,
|
||||
`false`,
|
||||
}
|
||||
for _, exprStr := range exprStrs {
|
||||
_, err := ParseExpr(helper, exprStr)
|
||||
|
|
|
@ -377,3 +377,7 @@ func IsAlwaysTruePlan(plan *planpb.PlanNode) bool {
|
|||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func canBeExecuted(e *ExprWithType) bool {
|
||||
return typeutil.IsBoolType(e.dataType) && !e.nodeDependent
|
||||
}
|
||||
|
|
|
@ -131,3 +131,46 @@ func TestIsAlwaysTruePlan(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
func Test_canBeExecuted(t *testing.T) {
|
||||
type args struct {
|
||||
e *ExprWithType
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
args args
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
args: args{
|
||||
e: &ExprWithType{
|
||||
dataType: schemapb.DataType_Int64,
|
||||
},
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
args: args{
|
||||
e: &ExprWithType{
|
||||
dataType: schemapb.DataType_Bool,
|
||||
nodeDependent: true,
|
||||
},
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
args: args{
|
||||
e: &ExprWithType{
|
||||
dataType: schemapb.DataType_Bool,
|
||||
nodeDependent: false,
|
||||
},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
assert.Equalf(t, tt.want, canBeExecuted(tt.args.e), "canBeExecuted(%v)", tt.args.e)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue