cel-go icon indicating copy to clipboard operation
cel-go copied to clipboard

math.MaxUInt cost reported for equality/inequality operations on both Timestamp and Duration

Open DangerOnTheRanger opened this issue 1 year ago • 0 comments

Describe the bug Directly comparing a pair of Timestamps or Durations using either == or != will result in an expression with an estimated cost of math.MaxUInt. Other operators (>, <=, etc.) are not affected, and will return a low constant cost as expected. As far as I can tell, this is because operators besides ==/!= have type-specific overloads and thus fall through to the default case of coster.functionCost.

But this is not so for ==/!=, which are handled by the same case that handles all equality/inequality checks. coster.sizeEstimate, called a bit further down, doesn't know how to handle Timestamps/Durations and returns math.MaxUInt. This is a similar issue to what https://github.com/google/cel-go/pull/571 fixes, though it's a bit different here - I think it should be fixable just by adding an extra overload on ==/!= for both Timestamps and Durations.

This should be resolved before a new release is cut to pick up #571 - I can take care of this issue as well (I'd self-assign but it seems I don't have the right permissions unfortunately).

To Reproduce Check which components this affects:

  • [ ] parser
  • [x] checker
  • [ ] interpreter

Sample expression and input that reproduces the issue:

timestamp1 == timestamp2
duration1 != duration2

Test setup (add the following tests to TestCost in cost_test.go):

{
	name: "timestamp equality check",
	expr: `timestamp1 == timestamp2`,
	decls: []*exprpb.Decl{
			decls.NewVar("timestamp1", decls.Timestamp),
			decls.NewVar("timestamp2", decls.Timestamp),
		},
	wanted: CostEstimate{Min: 3, Max: 3},
},
{
	name: "duration inequality check",
	expr: `duration1 != duration2`,
	decls: []*exprpb.Decl{
			decls.NewVar("duration1", decls.Duration),
			decls.NewVar("duration2", decls.Duration),
		},
	wanted: CostEstimate{Min: 3, Max: 3},
},

Expected behavior The tests should pass. However, on my system I get the following output:

--- FAIL: TestCost (0.14s)
    --- FAIL: TestCost/timestamp_equality_check (0.00s)
        cost_test.go:414: Got cost interval [3, 1844674407370955266], wanted [3, 3]
    --- FAIL: TestCost/duration_inequality_check (0.00s)
        cost_test.go:414: Got cost interval [3, 1844674407370955266], wanted [3, 3]
FAIL
exit status 1
FAIL    github.com/google/cel-go/checker        0.317s

Additional context cel-go revision: 25bb4c6da1759f0104ce8ee935431d32b71977b6

DangerOnTheRanger avatar Aug 11 '22 01:08 DangerOnTheRanger