EQL: Add match function implementation#55182
EQL: Add match function implementation#55182rw-access merged 9 commits intoelastic:masterfrom rw-access:eql/match-function
Conversation
|
Pinging @elastic/es-ql (:Query Languages/EQL) |
x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java
Outdated
Show resolved
Hide resolved
...n/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/EqlFunctionRegistry.java
Outdated
Show resolved
Hide resolved
astefan
left a comment
There was a problem hiding this comment.
I would like to see more tests for this function and properly deal with matchLite (see comment review).
| error("process where between(process_name, \"s\", \"e\", false, 2)")); | ||
| } | ||
|
|
||
| public void testMatchWithText() { |
x-pack/plugin/eql/qa/common/src/main/resources/test_queries_supported.toml
Show resolved
Hide resolved
x-pack/plugin/eql/qa/common/src/main/resources/test_queries_supported.toml
Show resolved
Hide resolved
| } | ||
|
|
||
| public void testCIDRMatchNonIPField() { | ||
| public void testCIDRMatchAgainstField() { |
There was a problem hiding this comment.
I rearranged these test methods alphabetically in hopes that it makes git conflicts less likely
aleksmaus
left a comment
There was a problem hiding this comment.
couple of comments, otherwise LGTM
| String msg = e.getMessage(); | ||
| assertEquals("Found 1 problem\n" + | ||
| "line 1:15: second argument of [match(process_name, 1)] " + | ||
| "must be [string], found value [1] type [integer]", msg); |
There was a problem hiding this comment.
is there a test where match is passed only one argument?
There was a problem hiding this comment.
what about?
process where match(process_name, null)
| def(EndsWith.class, EndsWith::new, "endswith"), | ||
| def(IndexOf.class, IndexOf::new, "indexof"), | ||
| def(Length.class, Length::new, "length"), | ||
| def(Match.class, Match::new, "match", "matchlite"), |
There was a problem hiding this comment.
I originally had "matchLite" but apparently the aliases have to also be normalized to lowercase, so it's "matchlite"
both functions have been around for a while, but matchLite was more limited than regex -- had character clasess and *, *?, and + because of our underlying implementation.
now, they both have the same functionality, so the alias is just for backwards compatibility.
* EQL: Add Match function * EQL: Add note about character classes * EQL: QueryFolderFailTests.java * EQL: Add match() fail tests * EQL: Add match tests and fix alias * EQL: Add match verifier failure tests * EQL: Reorder query folder fail tests
|
7.x backport 6da686c |
Closes #55178
Discovered that per https://www.elastic.co/guide/en/elasticsearch/reference/current/regexp-syntax.html, character classes aren't supported.
@jrodewig I think this may be worth noting in SQL and EQL docs.