Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow modifying the AST in Parse time #366

Open
2 of 3 tasks
glerchundi opened this issue May 28, 2020 · 1 comment
Open
2 of 3 tasks

Allow modifying the AST in Parse time #366

glerchundi opened this issue May 28, 2020 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@glerchundi
Copy link

Feature request checklist

  • There are no issues that match the desired change
  • The change is large enough it can't be addressed with a simple Pull Request
  • If this is a bug, please file a Bug Report.

I think it's preferable to open an issue instead of a PR because:

  • We don't have time to address it right now and it's gonna be useful for tracking purposes
  • It probably requires discussion

Change

@lopezator asked in #359 if not checking the sanity of values inside known function overloads in Compile time was an expected behaviour. And it was, there is no constant value checking meaning that Compile (Parse+Check) won't complain about these kind of expressions: birth_date >= timestamp("invalid-date!").

Although @TristonianJones said that there would be a possibility in the future to include linting rules to Compile that doesn't fully solve our problem. We're using CEL to parse (& check!) the filter (https://google.aip.dev/160) attribute in our List API calls (https://google.aip.dev/132). Then with the resulting AST we convert it into our WHERE clause in the SQL SELECT statement.

The point is that if the AST is left unmodified the AST->SQL WHERE (or whatever filtering language) converting layers gets more complicated than they should because they need to take care about those intermediate functions by themselves.

Ideally for us, Compile would have an option to contract those constants and optimize the AST. But it could also be possible to achieve exactly the same optimized AST with the decorators thing @TristonianJones proposed here and letting cel-go users by writing that logic outside of the library.

Alternatives considered

Using cel.Program with Eval in combination of OptOptimize but it would only check if the expression is valid, the original AST unmodified.

@TristonianJones TristonianJones added the enhancement New feature or request label May 28, 2020
@TristonianJones
Copy link
Collaborator

As a first step, the plan is to introduce validators (#762) to allow for custom AST validations to be run after type-check. Once the exprpb.Expr is migrated to a Go-native set of types, we'll expose optimizers which will allow you to constant fold or mutate the AST after the type-check and validators run. These features will only be available for type-checked expressions.

@TristonianJones TristonianJones self-assigned this Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants