From 77bbe9910243ed9e8c83ae066a619c95308120a3 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Fri, 11 Aug 2017 22:28:47 +0200 Subject: [PATCH] Fix two unreleased percolator query analyze bugs * If in a range query upper is smaller than lower then ignore the range query * If two empty range extractions are compared don't fail with NoSuchElementException --- .../percolator/QueryAnalyzer.java | 18 +++++++++++++-- .../percolator/QueryAnalyzerTests.java | 22 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/QueryAnalyzer.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/QueryAnalyzer.java index e398886a45e..2b65b1987e4 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/QueryAnalyzer.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/QueryAnalyzer.java @@ -45,6 +45,7 @@ import org.apache.lucene.search.spans.SpanQuery; import org.apache.lucene.search.spans.SpanTermQuery; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.NumericUtils; +import org.apache.lucene.util.UnicodeUtil; import org.elasticsearch.common.logging.LoggerMessageFormat; import org.elasticsearch.common.lucene.search.function.FunctionScoreQuery; @@ -361,6 +362,13 @@ final class QueryAnalyzer { byte[] lowerPoint = pointRangeQuery.getLowerPoint(); byte[] upperPoint = pointRangeQuery.getUpperPoint(); + + // Need to check whether upper is not smaller than lower, otherwise NumericUtils.subtract(...) fails IAE + // If upper is really smaller than lower then we deal with like MatchNoDocsQuery. (verified and no extractions) + if (new BytesRef(lowerPoint).compareTo(new BytesRef(upperPoint)) > 0) { + return new Result(true, Collections.emptySet()); + } + byte[] interval = new byte[16]; NumericUtils.subtract(16, 0, prepad(upperPoint), prepad(lowerPoint), interval); return new Result(false, Collections.singleton(new QueryExtraction( @@ -453,6 +461,12 @@ final class QueryAnalyzer { if (onlyRangeBasedExtractions) { BytesRef extraction1SmallestRange = smallestRange(filtered1); BytesRef extraction2SmallestRange = smallestRange(filtered2); + if (extraction1SmallestRange == null) { + return extractions2; + } else if (extraction2SmallestRange == null) { + return extractions1; + } + // Keep the clause with smallest range, this is likely to be the rarest. if (extraction1SmallestRange.compareTo(extraction2SmallestRange) <= 0) { return extractions1; @@ -500,10 +514,10 @@ final class QueryAnalyzer { } private static BytesRef smallestRange(Set terms) { - BytesRef min = terms.iterator().next().range.interval; + BytesRef min = null; for (QueryExtraction qt : terms) { if (qt.range != null) { - if (qt.range.interval.compareTo(min) < 0) { + if (min == null || qt.range.interval.compareTo(min) < 0) { min = qt.range.interval; } } diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryAnalyzerTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryAnalyzerTests.java index 81856a1d213..ff70c664460 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryAnalyzerTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryAnalyzerTests.java @@ -589,6 +589,21 @@ public class QueryAnalyzerTests extends ESTestCase { queryTerms2 = terms(new int[]{2, 3, 4}, "1", "456"); result = selectBestExtraction(Collections.emptyMap(), queryTerms1, queryTerms2); assertSame("Ignoring ranges, so then prefer queryTerms1, because it has the longest shortest term", queryTerms1, result); + + queryTerms1 = terms(new int[]{}); + queryTerms2 = terms(new int[]{}); + result = selectBestExtraction(Collections.emptyMap(), queryTerms1, queryTerms2); + assertSame("In case query extractions are empty", queryTerms2, result); + + queryTerms1 = terms(new int[]{1}); + queryTerms2 = terms(new int[]{}); + result = selectBestExtraction(Collections.emptyMap(), queryTerms1, queryTerms2); + assertSame("In case query a single extraction is empty", queryTerms1, result); + + queryTerms1 = terms(new int[]{}); + queryTerms2 = terms(new int[]{1}); + result = selectBestExtraction(Collections.emptyMap(), queryTerms1, queryTerms2); + assertSame("In case query a single extraction is empty", queryTerms2, result); } public void testSelectBestExtraction_boostFields() { @@ -753,6 +768,13 @@ public class QueryAnalyzerTests extends ESTestCase { expectThrows(UnsupportedQueryException.class, () -> analyze(query2, Collections.emptyMap())); } + public void testPointRangeQuery_lowerUpperReversed() { + Query query = IntPoint.newRangeQuery("_field", 20, 10); + Result result = analyze(query, Collections.emptyMap()); + assertTrue(result.verified); + assertThat(result.extractions.size(), equalTo(0)); + } + public void testIndexOrDocValuesQuery() { Query query = new IndexOrDocValuesQuery(IntPoint.newRangeQuery("_field", 10, 20), SortedNumericDocValuesField.newSlowRangeQuery("_field", 10, 20));