-
Notifications
You must be signed in to change notification settings - Fork 25.2k
EQL: Introduce case insensitive variant in~ #68176
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
Conversation
Complement to in, in~ does case-insensitive matching without any pattern matching - : should be used instead. process where name in~ ("ExPLorEr.eXe") will match name against explorer.exe regardless of case. Fix elastic#68172
Pinging @elastic/es-ql (Team:QL) |
@@ -167,8 +168,13 @@ public Expression visitOperatorExpressionDefault(EqlBaseParser.OperatorExpressio | |||
switch (predicate.kind.getType()) { | |||
case EqlBaseParser.SEQ: | |||
return Predicates.combineOr(expressions(predicate.constant()).stream() | |||
.map(c -> new InsensitiveWildcardEquals(source, expr, c, zoneId)) |
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.
Is this correct? shouldn't this be for the IN_INSESITIVE
and for SEQ
the InsensitiveEquals
?
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.
No. See the tests - :
or SEQ supports wildcards while in~
does not, hence why it is only insensitive.
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.
Oops, sorry, I confused myself.
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.
LGTM
import org.antlr.v4.runtime.CharStream; | ||
import org.antlr.v4.runtime.Token; | ||
import org.antlr.v4.runtime.TokenStream; | ||
import org.antlr.v4.runtime.*; |
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.
Please revert star imports.
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.
That looks like a side-effect on ANTLR generation.
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.
LGTM
String wildString = null; | ||
if (cmp instanceof InsensitiveWildcardEquals || cmp instanceof InsensitiveWildcardNotEquals) { | ||
// expr : "wildcard*phrase?" || expr !: "wildcard*phrase?" | ||
Expression result = cmp; |
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.
Seems like if you move this outside the branch you could remove the inner return.
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.
LGTM
/cc @jrodewig |
Complement to in, in~ does case-insensitive matching without any
pattern matching - : should be used instead.
process where name in~ ("ExPLorEr.eXe")
will match name against explorer.exe regardless of case.
Fix #68172