Optimize type conversion functions.

[TristonianJones]

Unary type-conversion functions with literal arguments should be evaluated
eagerly when the cel.EvalOptions(cel.OptOptimize) flag is set. The conversion
will also ensure that if an error is encountered when evaluating these functions,
such as when constructing timestamps or durations from strings, the evaluation
will result in an error in the env.Program call.

This is not a perfect fix for #359, but it can be used to assist with correctness checks.

[glerchundi]

This is great! Correct me if i'm wrong, this implementation would contract the AST from a timestamp('correct-ts-value') into *exprpb.Constant_TimestampValue?

[lopezator]

Yahoo! 🎉

[TristonianJones]

@glerchundi well, not exactly. The code will convert from timestamp(<rfc_3339_str>) to a CEL ref.Val. The AST is left unmodified which is why this is an imperfect fix for your needs. You could use it as a work-around in the short term.

[glerchundi]

Ok, thanks anyway :D. Good job!

[TristonianJones]

@glerchundi I'll eventually get to supporting linting. This will at least tell you that the checked-AST has timestamp(<str>) and duration(<str>) calls with well-formed string inputs. It's not perfect, but it should meet your needs in the short-term. Just make sure to turn on optimization.

I have a few more tests that I need to add to catch malformed inputs at optimization time, but then I'll send this out for review.

[glerchundi]

I guess it does partially.

Currently we're parsing the expression and the walking the AST to build the SQL expression. Having an evaluation process where it executes the corresponding sanity check before proceeding sounds good but it's not enough as we need to continue walking the unmodified AST and taking care of the call expressions in it (like timestamp).

Ideally I would expect OptOptimize to construct (i don't care the mechanism newAst := PruneAst(details, oldAst)) a cleaned up tree in order to avoid dealing with convert overloads on our side.

We're already doing it but makes the implementation more complicated that I would like it to be.

[glerchundi]

Does your decorator implementation in #363 helps converting/contracting ref.Val (with timestamp(const)) into another ref.Val (with timestamp_const_value). I must admit that i'm a bit lost there :D

[TristonianJones]

Hi @glerchundi, #363 is a decorator that participates in an AST walk done before evaluation. It's not really what you're looking for since it doesn't modify the AST; however, what I could do is expose a ParserDecorator which would allow you to observe, alter, or replace the Expr AST nodes at parse time. It would at least simplify your existing logic.

[TristonianJones]

PTAL

[glerchundi]

That is a fantastic idea @TristonianJones!

I'll create an issue asking for exactly that and in case we found a free time slot & we see ourselves with the capacity to make it happen without messing the code we'll send a PR.