diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/Criterion.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/Criterion.java index b95ff03849e..1897324feea 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/Criterion.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/Criterion.java @@ -25,7 +25,7 @@ public class Criterion { private final boolean descending; - Criterion(int stage, + public Criterion(int stage, Q queryRequest, List keys, HitExtractor timestamp, @@ -83,7 +83,7 @@ public class Criterion { if (tiebreaker != null) { Object tb = tiebreaker.extract(hit); - if (tb instanceof Comparable == false) { + if (tb != null && tb instanceof Comparable == false) { throw new EqlIllegalArgumentException("Expected tiebreaker to be Comparable but got {}", tb); } tbreaker = (Comparable) tb; diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/Ordinal.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/Ordinal.java index ff72d89580d..126e95db2fb 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/Ordinal.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/Ordinal.java @@ -58,17 +58,15 @@ public class Ordinal implements Comparable { } if (timestamp == o.timestamp) { if (tiebreaker != null) { - if (o.tiebreaker != null) { - return tiebreaker.compareTo(o.tiebreaker); - } - // nulls are first - lower than any other value - // other tiebreaker is null this one isn't, fall through 1 + // if the other tiebreaker is null, it is higher (nulls are last) + return o.tiebreaker != null ? tiebreaker.compareTo(o.tiebreaker) : -1; } - // null tiebreaker + // this tiebreaker is null else { - if (o.tiebreaker != null) { - return -1; - } else { + // nulls are last so unless both are null (equal) + // this ordinal is greater (after) then the other tiebreaker + // so fall through to 1 + if (o.tiebreaker == null) { return 0; } } diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/CriterionOrdinalExtractionTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/CriterionOrdinalExtractionTests.java new file mode 100644 index 00000000000..89e4dc9b37e --- /dev/null +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/CriterionOrdinalExtractionTests.java @@ -0,0 +1,105 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.eql.execution.search; + +import org.elasticsearch.common.document.DocumentField; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.text.Text; +import org.elasticsearch.search.SearchHit; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.eql.EqlIllegalArgumentException; +import org.elasticsearch.xpack.eql.execution.assembler.BoxedQueryRequest; +import org.elasticsearch.xpack.eql.execution.assembler.Criterion; +import org.elasticsearch.xpack.eql.execution.search.extractor.FieldHitExtractor; +import org.elasticsearch.xpack.ql.execution.search.extractor.HitExtractor; +import org.elasticsearch.xpack.ql.type.DataTypes; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; + +public class CriterionOrdinalExtractionTests extends ESTestCase { + private String tsField = "timestamp"; + private String tbField = "tiebreaker"; + + private HitExtractor tsExtractor = new FieldHitExtractor(tsField, tsField, DataTypes.LONG, null, true, null, false); + private HitExtractor tbExtractor = new FieldHitExtractor(tbField, tbField, DataTypes.LONG, null, true, null, false); + + public void testTimeOnly() throws Exception { + long time = randomLong(); + Ordinal ordinal = ordinal(searchHit(time, null), false); + assertEquals(time, ordinal.timestamp()); + assertNull(ordinal.tiebreaker()); + } + + public void testTimeAndTiebreaker() throws Exception { + long time = randomLong(); + long tb = randomLong(); + Ordinal ordinal = ordinal(searchHit(time, tb), true); + assertEquals(time, ordinal.timestamp()); + assertEquals(tb, ordinal.tiebreaker()); + } + + public void testTimeAndTiebreakerNull() throws Exception { + long time = randomLong(); + Ordinal ordinal = ordinal(searchHit(time, null), true); + assertEquals(time, ordinal.timestamp()); + assertNull(ordinal.tiebreaker()); + } + + public void testTimeNotComparable() throws Exception { + HitExtractor badExtractor = new FieldHitExtractor(tsField, tsField, DataTypes.BINARY, null, true, null, false); + SearchHit hit = searchHit(randomAlphaOfLength(10), null); + Criterion criterion = new Criterion(0, null, emptyList(), badExtractor, null, false); + EqlIllegalArgumentException exception = expectThrows(EqlIllegalArgumentException.class, () -> criterion.ordinal(hit)); + assertTrue(exception.getMessage().startsWith("Expected timestamp")); + } + + public void testTiebreakerNotComparable() throws Exception { + final Object o = randomDateTimeZone(); + HitExtractor badExtractor = new HitExtractor() { + @Override + public Object extract(SearchHit hit) { + return o; + } + + @Override + public String hitName() { + return null; + } + + @Override + public String getWriteableName() { + return null; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + } + }; + SearchHit hit = searchHit(randomLong(), o); + Criterion criterion = new Criterion(0, null, emptyList(), tsExtractor, badExtractor, false); + EqlIllegalArgumentException exception = expectThrows(EqlIllegalArgumentException.class, () -> criterion.ordinal(hit)); + assertTrue(exception.getMessage().startsWith("Expected tiebreaker")); + } + + private SearchHit searchHit(Object timeValue, Object tiebreakerValue) { + Map fields = new HashMap<>(); + fields.put(tsField, new DocumentField(tsField, singletonList(timeValue))); + fields.put(tbField, new DocumentField(tsField, singletonList(tiebreakerValue))); + + return new SearchHit(randomInt(), randomAlphaOfLength(10), new Text("_doc"), fields, emptyMap()); + } + + private Ordinal ordinal(SearchHit hit, boolean withTiebreaker) { + return new Criterion(0, null, emptyList(), tsExtractor, withTiebreaker ? tbExtractor : null, false).ordinal(hit); + } +} diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/OrdinalTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/OrdinalTests.java new file mode 100644 index 00000000000..0522abd57f4 --- /dev/null +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/search/OrdinalTests.java @@ -0,0 +1,107 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.eql.execution.search; + +import org.elasticsearch.test.ESTestCase; + +@SuppressWarnings("unchecked") +public class OrdinalTests extends ESTestCase { + + public void testCompareToDifferentTs() { + Ordinal one = new Ordinal(randomLong(), (Comparable) randomLong()); + Ordinal two = new Ordinal(randomLong(), (Comparable) randomLong()); + + assertEquals(Long.valueOf(one.timestamp()).compareTo(two.timestamp()), one.compareTo(two)); + } + + public void testCompareToSameTsDifferentTie() { + Long ts = randomLong(); + Ordinal one = new Ordinal(ts, (Comparable) randomLong()); + Ordinal two = new Ordinal(ts, (Comparable) randomLong()); + + assertEquals(one.tiebreaker().compareTo(two.tiebreaker()), one.compareTo(two)); + } + + public void testCompareToSameTsOneTieNull() { + Long ts = randomLong(); + Ordinal one = new Ordinal(ts, (Comparable) randomLong()); + Ordinal two = new Ordinal(ts, null); + + assertEquals(-1, one.compareTo(two)); + } + + @SuppressWarnings("rawtypes") + public void testCompareToSameTsSameTie() { + Long ts = randomLong(); + Comparable c = randomLong(); + Ordinal one = new Ordinal(ts, c); + Ordinal two = new Ordinal(ts, c); + + assertEquals(0, one.compareTo(two)); + assertEquals(0, one.compareTo(one)); + assertEquals(0, two.compareTo(two)); + } + + public void testCompareToSameTsSameTieNull() { + Long ts = randomLong(); + Ordinal one = new Ordinal(ts, null); + Ordinal two = new Ordinal(ts, null); + + assertEquals(0, one.compareTo(two)); + assertEquals(0, one.compareTo(one)); + assertEquals(0, two.compareTo(two)); + } + + public void testTestBetween() { + Ordinal before = new Ordinal(randomLongBetween(1000, 2000), (Comparable) randomLong()); + Ordinal between = new Ordinal(randomLongBetween(3000, 4000), (Comparable) randomLong()); + Ordinal after = new Ordinal(randomLongBetween(5000, 6000), (Comparable) randomLong()); + + assertTrue(before.between(before, after)); + assertTrue(after.between(before, after)); + assertTrue(between.between(before, after)); + + assertFalse(new Ordinal(randomLongBetween(0, 999), null).between(before, after)); + assertFalse(new Ordinal(randomLongBetween(7000, 8000), null).between(before, after)); + } + + public void testTestBefore() { + Ordinal before = new Ordinal(randomLongBetween(1000, 2000), (Comparable) randomLong()); + Ordinal after = new Ordinal(randomLongBetween(5000, 6000), (Comparable) randomLong()); + + assertTrue(before.before(after)); + assertFalse(before.before(before)); + assertFalse(after.before(before)); + } + + public void testBeforeOrAt() { + Ordinal before = new Ordinal(randomLongBetween(1000, 2000), (Comparable) randomLong()); + Ordinal after = new Ordinal(randomLongBetween(5000, 6000), (Comparable) randomLong()); + + assertTrue(before.beforeOrAt(after)); + assertTrue(before.beforeOrAt(before)); + assertFalse(after.beforeOrAt(before)); + } + + public void testTestAfter() { + Ordinal before = new Ordinal(randomLongBetween(1000, 2000), (Comparable) randomLong()); + Ordinal after = new Ordinal(randomLongBetween(5000, 6000), (Comparable) randomLong()); + + assertTrue(after.after(before)); + assertFalse(after.after(after)); + assertFalse(before.after(after)); + } + + public void testAfterOrAt() { + Ordinal before = new Ordinal(randomLongBetween(1000, 2000), (Comparable) randomLong()); + Ordinal after = new Ordinal(randomLongBetween(5000, 6000), (Comparable) randomLong()); + + assertTrue(after.afterOrAt(before)); + assertTrue(after.afterOrAt(after)); + assertFalse(before.afterOrAt(after)); + } +}