diff --git a/checker/cost.go b/checker/cost.go index 7312d1fe..16e5ecf8 100644 --- a/checker/cost.go +++ b/checker/cost.go @@ -340,6 +340,8 @@ func (c *coster) costSelect(e *exprpb.Expr) CostEstimate { sel := e.GetSelectExpr() var sum CostEstimate if sel.GetTestOnly() { + sum.Min = 1 + sum.Max = 1 return sum } sum = sum.Add(c.cost(sel.GetOperand())) diff --git a/checker/cost_test.go b/checker/cost_test.go index 4e8cb9b4..8948d690 100644 --- a/checker/cost_test.go +++ b/checker/cost_test.go @@ -75,7 +75,7 @@ func TestCost(t *testing.T) { name: "select: field test only", expr: `has(input.single_int32)`, decls: []*exprpb.Decl{decls.NewVar("input", decls.NewObjectType("google.expr.proto3.test.TestAllTypes"))}, - wanted: zeroCost, + wanted: CostEstimate{Min: 1, Max: 1}, }, { name: "estimated function call", diff --git a/interpreter/interpretable.go b/interpreter/interpretable.go index f9724d46..74aef257 100644 --- a/interpreter/interpretable.go +++ b/interpreter/interpretable.go @@ -110,11 +110,10 @@ type InterpretableConstructor interface { // Core Interpretable implementations used during the program planning phase. type evalTestOnly struct { - id int64 - attr InterpretableAttribute - qual Qualifier - field types.String - isProtoField bool + id int64 + attr InterpretableAttribute + qual Qualifier + field types.String } // ID implements the Interpretable interface method. diff --git a/interpreter/planner.go b/interpreter/planner.go index 165a388b..620f9aac 100644 --- a/interpreter/planner.go +++ b/interpreter/planner.go @@ -220,19 +220,11 @@ func (p *planner) planSelect(expr *exprpb.Expr) (Interpretable, error) { // Return the test only eval expression. if sel.GetTestOnly() { - isProtoField := false - if opType.GetMessageType() != "" { - ft, found := p.provider.FindFieldType(opType.GetMessageType(), sel.GetField()) - if found && ft.IsSet != nil && ft.GetFrom != nil { - isProtoField = true - } - } return &evalTestOnly{ - id: expr.GetId(), - field: types.String(sel.GetField()), - attr: attr, - qual: qual, - isProtoField: isProtoField, + id: expr.GetId(), + field: types.String(sel.GetField()), + attr: attr, + qual: qual, }, nil } diff --git a/interpreter/runtimecost.go b/interpreter/runtimecost.go index 33a4ea56..d355733b 100644 --- a/interpreter/runtimecost.go +++ b/interpreter/runtimecost.go @@ -38,12 +38,6 @@ type ActualCostEstimator interface { func CostObserver(tracker *CostTracker) EvalObserver { observer := func(id int64, programStep any, val ref.Val) { switch t := programStep.(type) { - case *evalTestOnly: - // Protobuf has() checks do not incur runtime cost due to not using Eval() - // in older versions of CEL; this keeps the runtime cost behavior consistent - if !t.isProtoField { - tracker.cost++ - } case ConstantQualifier: // TODO: Push identifiers on to the stack before observing constant qualifiers that apply to them // and enable the below pop. Once enabled this can case can be collapsed into the Qualifier case. @@ -75,6 +69,8 @@ func CostObserver(tracker *CostTracker) EvalObserver { tracker.stack.drop(t.rhs.ID(), t.lhs.ID()) case *evalFold: tracker.stack.drop(t.iterRange.ID()) + case *evalTestOnly: + tracker.cost += common.SelectAndIdentCost case Qualifier: tracker.cost++ case InterpretableCall: diff --git a/interpreter/runtimecost_test.go b/interpreter/runtimecost_test.go index 0bcd39e3..26a19368 100644 --- a/interpreter/runtimecost_test.go +++ b/interpreter/runtimecost_test.go @@ -306,7 +306,7 @@ func TestRuntimeCost(t *testing.T) { name: "select: field test only", expr: `has(input.single_int32)`, decls: []*exprpb.Decl{decls.NewVar("input", decls.NewObjectType("google.expr.proto3.test.TestAllTypes"))}, - want: 0, + want: 1, in: map[string]any{ "input": &proto3pb.TestAllTypes{ RepeatedBool: []bool{false},