EQL: Convert wildcards to LIKE in analyzer#51901
EQL: Convert wildcards to LIKE in analyzer#51901rw-access merged 4 commits intoelastic:masterfrom rw-access:eql/wildcards
Conversation
|
Pinging @elastic/es-search (:Search/EQL) |
There was a problem hiding this comment.
Can a wildcard appear in any type of string? e.g. some*glob?
I wonder whether the parser could detect it so instead of having Literal that might a string, it could have its own expression rule.
There was a problem hiding this comment.
A potential improvement is to check whether an expression is foldable instead of being a literal.
Thus if string concatenation were to be added, the rule would still be applied:
if (e.foldable() && e.fold() instanceof String) {
return e.fold().toString().contains("*");
}which can be transformed into a one-liner:
return e.foldable() && e.fold() instanceof String && e.fold().toString().contains("*");There was a problem hiding this comment.
what about foo > "wild*card" or other value comparisons?
If that's valid grammar, the verifier should pick the pattern and fail the query.
There was a problem hiding this comment.
yeah, it is valid grammar, but since it's not == or !=, this will just be a lexicographical comparison
There was a problem hiding this comment.
That would mean the operators are not consistent since == would expand the wildcard while > & co would compare against it...
There was a problem hiding this comment.
isWildcard already does the checks so simply do: eq.fold().toString()
x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/analysis/Analyzer.java
Outdated
Show resolved
Hide resolved
astefan
left a comment
There was a problem hiding this comment.
Left a comment regarding tests.
There was a problem hiding this comment.
Would be possible to add more tests that also look at scenarios involving escape characters and all types of string that eql supports?
Can the * be escaped? If so, we should have a test covering this case.
There was a problem hiding this comment.
yeah, I can add more tests. And there's no escape for *. But if you want to perform an exact (+/- case sensitivity) comparison, you can put the wildcard on the left (#51901 (comment)). Whether this functionality is good or not is a fair question, and I think it's fair to change this because I doubt any users are aware of the workaround.
field == "*wildcard*" <==> field LIKE "%wildcard%"
"*wildcard*" == field <==> field == '*wildcard*'
There was a problem hiding this comment.
To be honest, I find this more like a bug, rather than feature (that implies an workaround).
A == B and B == A should be equivalent imo. Equality is not a predicate (like LIKE) where a certain element sits on the right and another one sits on the left and they are not interchangeable.
|
I think it's worth discussing the feature and then figure out the implementation for it.
|
|
Agreed that wildcards should be made commutative. I don't think we should add a new construct like LIKE just yet. In the future, I think it could make sense, after we go through the usual deprecation steps. I think the main question I have is -- should this be a separate rule in the grammar, or should it be handled in the analyzer? |
|
If the wildcard can be picked up by the parser through a dedicated rule, I would opt for that. @andrei how close are you to moving the optimizer rules in? |
| @@ -33,6 +43,7 @@ protected Iterable<RuleExecutor<LogicalPlan>.Batch> batches() { | |||
| new BooleanSimplification(), | |||
| new BooleanLiteralsOnTheRight(), | |||
| // needs to occur before BinaryComparison combinations | |||
There was a problem hiding this comment.
The comment seems incorrect as it referred to PropagateEquals
There was a problem hiding this comment.
where should ReplaceWildcards() be moved to?
x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/optimizer/Optimizer.java
Show resolved
Hide resolved
|
elasticsearch-ci/2 is failing due to a checkstyle violation. |
* EQL: Convert wildcard comparisons to Like * EQL: Simplify wildcard handling, update tests * EQL: Lint fixes for Optimizer.java
Addressing the comment thread from #51558 (comment).
Added ReplaceWildcards to the optimizer which detects the
== "wild*card*"or!= "wild*card*"patterns and replaces with LIKE.This is branched from #51886, so only the last commit is relevant.Update: Resolves #53104