-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
enhancement(remap): add match_any function #7414
Conversation
lib/vrl/stdlib/src/match_any.rs
Outdated
Parameter { | ||
keyword: "patterns", | ||
kind: kind::ARRAY, | ||
required: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would this not be required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! It should definitely be required. Seems like this was somehow fixed when rebasing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doh, I was looking at somewhere else. Will push a fix now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, apart from I suspect the patterns parameter should be required.
It's a shame we have to convert the regexes to strings and back again, but I can't see another way around that. It is only at boot time, so not critical.
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me! I might add a note to the docs that using this is faster if you have multiple regexes to check than using match
multiple times.
I wonder if we should add a follow up issues to convert successive |
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
This was something I looked into first, but it seemed pretty non-trivial. I'll at least open an issue around adding that kind of optimization pass to the VRL compiler. |
This is currently built on top of #7250 to take advantage of some of @jszwedko additions there.
It's also not documented and has oneunwrap
that needs removed, but I'm opening it in this state to get some feedback since it's my first real work with VRL.The goal here is to optimize the case of checking against multiple regexes. Instead of
match(.foo, ...) || match(.foo, ...) || ...
, you can simply pass an array of patterns that are compiled to aRegexSet
and checked efficiently in one pass. This is also more ergonomic than asking users to combine patterns manually.