From 3bee28056f3283e4aec0436550562434f66566fc Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Tue, 29 Sep 2020 18:12:27 +0300 Subject: [PATCH] EQL: Fix bug in sequences with any pattern (#63007) Fix query creation inside sequences with any queries due to lacking a clause to combine, which lead to an invalid request being created. Fix #62967 (cherry picked from commit ff59d8823919a6e70928816e5c3687308ebde33f) --- .../test/eql/BaseEqlSpecTestCase.java | 4 +--- .../org/elasticsearch/test/eql/DataLoader.java | 10 +++++++++- .../test/eql/EqlExtraSpecTestCase.java | 5 +++++ .../elasticsearch/test/eql/EqlSpecTestCase.java | 5 +++++ .../main/resources/additional_test_queries.toml | 9 +++++++++ .../org/elasticsearch/xpack/eql/EqlSpecIT.java | 5 ----- .../execution/assembler/BoxedQueryRequest.java | 15 +++++++++------ 7 files changed, 38 insertions(+), 15 deletions(-) diff --git a/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/BaseEqlSpecTestCase.java b/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/BaseEqlSpecTestCase.java index e581cbbca2f..d774539ec03 100644 --- a/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/BaseEqlSpecTestCase.java +++ b/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/BaseEqlSpecTestCase.java @@ -175,7 +175,5 @@ public abstract class BaseEqlSpecTestCase extends ESRestTestCase { return true; } - protected String sequenceField() { - return "sequence"; - } + protected abstract String sequenceField(); } diff --git a/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/DataLoader.java b/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/DataLoader.java index 3226de9291d..4830f56898f 100644 --- a/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/DataLoader.java +++ b/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/DataLoader.java @@ -57,6 +57,9 @@ public class DataLoader { private static final long FILETIME_EPOCH_DIFF = 11644473600000L; private static final long FILETIME_ONE_MILLISECOND = 10 * 1000; + // runs as java main + private static boolean main = false; + private static Map getReplacementPatterns() { final Map map = new HashMap<>(1); map.put("[runtime_random_keyword_type]", new String[] {"keyword", "wildcard"}); @@ -64,6 +67,7 @@ public class DataLoader { } public static void main(String[] args) throws IOException { + main = true; try (RestClient client = RestClient.builder(new HttpHost("localhost", 9200)).build()) { loadDatasetIntoEs(new RestHighLevelClient( client, @@ -121,7 +125,7 @@ public class DataLoader { while ((line = reader.readLine()) != null) { if (line.startsWith("#") == false) { for (Entry entry : replacementPatterns.entrySet()) { - line = line.replace(entry.getKey(), ESRestTestCase.randomFrom(entry.getValue())); + line = line.replace(entry.getKey(), randomOf(entry.getValue())); } b.append(line); } @@ -130,6 +134,10 @@ public class DataLoader { } } + private static CharSequence randomOf(String...values) { + return main ? values[0] : ESRestTestCase.randomFrom(values); + } + @SuppressWarnings("unchecked") private static void loadData(RestHighLevelClient client, String indexName, boolean winfileTime, URL resource, CheckedBiFunction p) diff --git a/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/EqlExtraSpecTestCase.java b/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/EqlExtraSpecTestCase.java index 7abb5855c20..a7d69167992 100644 --- a/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/EqlExtraSpecTestCase.java +++ b/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/EqlExtraSpecTestCase.java @@ -23,4 +23,9 @@ public abstract class EqlExtraSpecTestCase extends BaseEqlSpecTestCase { public EqlExtraSpecTestCase(String query, String name, long[] eventIds, boolean caseSensitive) { super(TEST_EXTRA_INDEX, query, name, eventIds, caseSensitive); } + + @Override + protected String sequenceField() { + return "sequence"; + } } diff --git a/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/EqlSpecTestCase.java b/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/EqlSpecTestCase.java index 81beec690ad..087a61a4ae9 100644 --- a/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/EqlSpecTestCase.java +++ b/x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/EqlSpecTestCase.java @@ -46,6 +46,11 @@ public abstract class EqlSpecTestCase extends BaseEqlSpecTestCase { return asArray(filteredSpecs); } + @Override + protected String sequenceField() { + return "serial_event_id"; + } + public EqlSpecTestCase(String query, String name, long[] eventIds, boolean caseSensitive) { super(TEST_INDEX, query, name, eventIds, caseSensitive); } diff --git a/x-pack/plugin/eql/qa/common/src/main/resources/additional_test_queries.toml b/x-pack/plugin/eql/qa/common/src/main/resources/additional_test_queries.toml index e9f6595955b..25e478adabe 100644 --- a/x-pack/plugin/eql/qa/common/src/main/resources/additional_test_queries.toml +++ b/x-pack/plugin/eql/qa/common/src/main/resources/additional_test_queries.toml @@ -253,3 +253,12 @@ sequence [process where true] by unique_ppid ''' expected_event_ids = [] + +[[queries]] +name = "sequenceWithAnyFilter" +query = ''' +sequence + [any where serial_event_id<3] by unique_pid + [any where true] by unique_ppid +''' +expected_event_ids = [1, 2, 2, 3] diff --git a/x-pack/plugin/eql/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlSpecIT.java b/x-pack/plugin/eql/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlSpecIT.java index 6d43f4a265a..d84668d16c1 100644 --- a/x-pack/plugin/eql/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlSpecIT.java +++ b/x-pack/plugin/eql/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlSpecIT.java @@ -13,9 +13,4 @@ public class EqlSpecIT extends EqlSpecTestCase { public EqlSpecIT(String query, String name, long[] eventIds, boolean caseSensitive) { super(query, name, eventIds, caseSensitive); } - - @Override - protected String sequenceField() { - return "serial_event_id"; - } } diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/BoxedQueryRequest.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/BoxedQueryRequest.java index 5908e1ec91f..62b324636a9 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/BoxedQueryRequest.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/BoxedQueryRequest.java @@ -18,7 +18,7 @@ import static org.elasticsearch.index.query.QueryBuilders.rangeQuery; /** * Ranged or boxed query. Provides a beginning or end to the current query. * The query moves between them through search_after. - * + * * Note that the range is not set at once on purpose since each query tends to have * its own number of results separate from the others. * As such, each query starts where it lefts to reach the current in-progress window @@ -35,8 +35,6 @@ public class BoxedQueryRequest implements QueryRequest { private Ordinal after; public BoxedQueryRequest(QueryRequest original, String timestamp, String tiebreaker) { - searchSource = original.searchSource(); - // setup range queries and preserve their reference to simplify the update timestampRange = rangeQuery(timestamp).timeZone("UTC").format("epoch_millis"); BoolQueryBuilder filter = boolQuery().filter(timestampRange); @@ -46,8 +44,13 @@ public class BoxedQueryRequest implements QueryRequest { } else { tiebreakerRange = null; } - // add ranges to existing query - searchSource.query(filter.must(searchSource.query())); + + searchSource = original.searchSource(); + // combine with existing query (if it exists) + if (searchSource.query() != null) { + filter = filter.must(searchSource.query()); + } + searchSource.query(filter); } @Override @@ -104,4 +107,4 @@ public class BoxedQueryRequest implements QueryRequest { private static String string(Ordinal o) { return o != null ? o.toString() : ""; } -} \ No newline at end of file +}