Skip to content

Commit 1bf2472

Browse files
Minor update on cost order (#1119)
* slight reordering in cost computation to preserve legacy compatibility * Additional tests to attempt to catch ordering issues with cost estimation
1 parent fb3fe56 commit 1bf2472

File tree

2 files changed

+68
-3
lines changed

2 files changed

+68
-3
lines changed

checker/cost.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -930,6 +930,9 @@ func (c *coster) computeSize(e ast.Expr) *SizeEstimate {
930930
if size, ok := c.computedSizes[e.ID()]; ok {
931931
return &size
932932
}
933+
if size := computeExprSize(e); size != nil {
934+
return size
935+
}
933936
// Ensure size estimates are computed first as users may choose to override the costs that
934937
// CEL would otherwise ascribe to the type.
935938
node := astNode{expr: e, path: c.getPath(e), t: c.getType(e)}
@@ -938,9 +941,6 @@ func (c *coster) computeSize(e ast.Expr) *SizeEstimate {
938941
c.computedSizes[e.ID()] = *size
939942
return size
940943
}
941-
if size := computeExprSize(e); size != nil {
942-
return size
943-
}
944944
if size := computeTypeSize(c.getType(e)); size != nil {
945945
return size
946946
}

checker/cost_test.go

+65
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,31 @@ func TestCost(t *testing.T) {
715715
expr: `self.val1 == 1.0`,
716716
wanted: FixedCostEstimate(3),
717717
},
718+
{
719+
name: "bytes list max",
720+
expr: "[bytes('012345678901'), bytes('012345678901'), bytes('012345678901'), bytes('012345678901'), bytes('012345678901')].max()",
721+
options: []CostOption{
722+
OverloadCostEstimate("list_bytes_max",
723+
func(estimator CostEstimator, target *AstNode, args []AstNode) *CallEstimate {
724+
if target != nil {
725+
// Charge 1 cost for comparing each element in the list
726+
elCost := CostEstimate{Min: 1, Max: 1}
727+
// If the list contains strings or bytes, add the cost of traversing all the strings/bytes as a way
728+
// of estimating the additional comparison cost.
729+
if elNode := listElementNode(*target); elNode != nil {
730+
k := elNode.Type().Kind()
731+
if k == types.StringKind || k == types.BytesKind {
732+
sz := sizeEstimate(estimator, elNode)
733+
elCost = elCost.Add(sz.MultiplyByCostFactor(common.StringTraversalCostFactor))
734+
}
735+
return &CallEstimate{CostEstimate: sizeEstimate(estimator, *target).MultiplyByCost(elCost)}
736+
}
737+
}
738+
return nil
739+
}),
740+
},
741+
wanted: CostEstimate{Min: 25, Max: 35},
742+
},
718743
}
719744

720745
for _, tst := range cases {
@@ -745,6 +770,14 @@ func TestCost(t *testing.T) {
745770
if err != nil {
746771
t.Fatalf("environment creation error: %v", err)
747772
}
773+
maxFunc, _ := decls.NewFunction("max",
774+
decls.MemberOverload("list_bytes_max",
775+
[]*types.Type{types.NewListType(types.BytesType)},
776+
types.BytesType))
777+
err = e.AddFunctions(maxFunc)
778+
if err != nil {
779+
t.Fatalf("environment creation error: %v", err)
780+
}
748781
err = e.AddIdents(tc.vars...)
749782
if err != nil {
750783
t.Fatalf("environment creation error: %s\n", err)
@@ -773,6 +806,9 @@ func (tc testCostEstimator) EstimateSize(element AstNode) *SizeEstimate {
773806
if l, ok := tc.hints[strings.Join(element.Path(), ".")]; ok {
774807
return &SizeEstimate{Min: 0, Max: l}
775808
}
809+
if element.Type() == types.BytesType {
810+
return &SizeEstimate{Min: 0, Max: 12}
811+
}
776812
return nil
777813
}
778814

@@ -793,3 +829,32 @@ func estimateSize(estimator CostEstimator, node AstNode) SizeEstimate {
793829
}
794830
return SizeEstimate{Min: 0, Max: math.MaxUint64}
795831
}
832+
833+
func listElementNode(list AstNode) AstNode {
834+
if params := list.Type().Parameters(); len(params) > 0 {
835+
lt := params[0]
836+
nodePath := list.Path()
837+
if nodePath != nil {
838+
// Provide path if we have it so that a OpenAPIv3 maxLength validation can be looked up, if it exists
839+
// for this node.
840+
path := make([]string, len(nodePath)+1)
841+
copy(path, nodePath)
842+
path[len(nodePath)] = "@items"
843+
return &astNode{path: path, t: lt, expr: nil}
844+
} else {
845+
// Provide just the type if no path is available so that worst case size can be looked up based on type.
846+
return &astNode{t: lt, expr: nil}
847+
}
848+
}
849+
return nil
850+
}
851+
852+
func sizeEstimate(estimator CostEstimator, t AstNode) SizeEstimate {
853+
if sz := t.ComputedSize(); sz != nil {
854+
return *sz
855+
}
856+
if sz := estimator.EstimateSize(t); sz != nil {
857+
return *sz
858+
}
859+
return SizeEstimate{Min: 0, Max: math.MaxUint64}
860+
}

0 commit comments

Comments
 (0)