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

Migrate cel.Ast to be a thin layer on ast.AST #806

Merged
merged 2 commits into from
Aug 15, 2023

Conversation

TristonianJones
Copy link
Collaborator

Code cleanup related to #789

common/ast/ast.go Outdated Show resolved Hide resolved
@TristonianJones
Copy link
Collaborator Author

PTAL

@TristonianJones TristonianJones merged commit 51cf846 into google:master Aug 15, 2023
2 checks passed
@TristonianJones TristonianJones deleted the migrate-cel-ast branch August 15, 2023 23:30
@tonyhb
Copy link

tonyhb commented Sep 23, 2023

@TristonianJones would it be possible to grab the *ast.AST from *cel.Ast? I'd like to walk the expressions to do some, well, things to it :)

@TristonianJones
Copy link
Collaborator Author

@tonyhb are you able to use the new Optimizer interfaces? They'll allow you to access the internal AST. In the future, I'll replace cel.Ast, but it's a bit of a compatibility challenge at the moment and I haven't had a chance to dig in.

@tonyhb
Copy link

tonyhb commented Sep 23, 2023

I had just implemnted the constant folding optimizer and noticed that it's Optimize call gets an *ast.AST, and that the static optimizer copies *Ast.impl direclty into *ast.AST — so I think that should do it. Thanks for the heads up.

I think you might be interested in why we need this:

We're compiling arbitrary expressions, and matching on arbitrary events (similar to pub/sub where this is used). The main difference is that expressions can be dynamically created, leading to a factorial combination of expression checking. Each event received must check every expression for matches; any new event or expression saved compounds expression checking.

The only way to simplify this is if we explicitly dont evaluate each event. It's a pain.

The plan is: parse the AST and check each predicate. For each predicate, create and merge the expression into a specific tree type for quick evaluation.

EG: If the expression tests string equality, throw the string match into an adaptive radix tree, where the leafs point to its expression. This way we can filter out all invalid expressions without having to test each one. We'll have to do the same for LT/GT on ints, etc.

It's an early plan right now, and we've only literally just started research on it.

Anyway, thanks again for the help!

@TristonianJones
Copy link
Collaborator Author

@tonyhb I think what you want is the disjunctive normal form of the expression to compute a trie / graph of the predicates and interested parties. This relates to our own thoughts related to expression set optimizers. Keep me posted! This sounds exciting

@tonyhb
Copy link

tonyhb commented Sep 23, 2023

Yep, literally that. Then merging all of the applicable trees into one set so that we can optimize which expressions need evaluating given input data. We just chatted about this last night for the first time, will let you know how investigation goes this weekend!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants