Skip to content

Commit

Permalink
Remove usage of IndexSortSortedNumericDocValuesRangeQuery (#61772)
Browse files Browse the repository at this point in the history 
This change reverts the optimization added in #56657.
We found a bug in `IndexSortSortedNumericDocValuesRangeQuery` that can fail the entire shard search request so this commit removes the optimization and restore the old behavior (range query on points) in this release branch.

Relates #61766
  • Loading branch information
@jimczi
jimczi authored Sep 1, 2020
1 parent 45518b7 commit 13b9536
Show file tree
Hide file tree
Showing 12 changed files with 145 additions and 310 deletions.
2 changes: 1 addition & 1 deletion ...es/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
}
hi = Math.round(Math.floor(dValue));
}
Query query = NumberFieldMapper.NumberType.LONG.rangeQuery(name(), lo, hi, true, true, hasDocValues(), context);
Query query = NumberFieldMapper.NumberType.LONG.rangeQuery(name(), lo, hi, true, true, hasDocValues());
if (boost() != 1f) {
query = new BoostQuery(query, boost());
}
Expand Down
26 changes: 13 additions & 13 deletions ...mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ public void testRangeQuery() throws IOException {
Double u = randomBoolean() ? null : (randomDouble() * 2 - 1) * 10000;
boolean includeLower = randomBoolean();
boolean includeUpper = randomBoolean();
Query doubleQ = NumberFieldMapper.NumberType.DOUBLE.rangeQuery("double", l, u, includeLower, includeUpper, false, MOCK_QSC);
Query scaledFloatQ = ft.rangeQuery(l, u, includeLower, includeUpper, MOCK_QSC);
Query doubleQ = NumberFieldMapper.NumberType.DOUBLE.rangeQuery("double", l, u, includeLower, includeUpper, false);
Query scaledFloatQ = ft.rangeQuery(l, u, includeLower, includeUpper, null);
assertEquals(searcher.count(doubleQ), searcher.count(scaledFloatQ));
}
IOUtils.close(reader, dir);
Expand All @@ -103,17 +103,17 @@ public void testRoundsUpperBoundCorrectly() {
= new ScaledFloatFieldMapper.ScaledFloatFieldType("scaled_float", 100);
Query scaledFloatQ = ft.rangeQuery(null, 0.1, true, false, MOCK_QSC);
assertEquals("scaled_float:[-9223372036854775808 TO 9]", scaledFloatQ.toString());
scaledFloatQ = ft.rangeQuery(null, 0.1, true, true, MOCK_QSC);
scaledFloatQ = ft.rangeQuery(null, 0.1, true, true, null);
assertEquals("scaled_float:[-9223372036854775808 TO 10]", scaledFloatQ.toString());
scaledFloatQ = ft.rangeQuery(null, 0.095, true, false, MOCK_QSC);
scaledFloatQ = ft.rangeQuery(null, 0.095, true, false, null);
assertEquals("scaled_float:[-9223372036854775808 TO 9]", scaledFloatQ.toString());
scaledFloatQ = ft.rangeQuery(null, 0.095, true, true, MOCK_QSC);
scaledFloatQ = ft.rangeQuery(null, 0.095, true, true, null);
assertEquals("scaled_float:[-9223372036854775808 TO 9]", scaledFloatQ.toString());
scaledFloatQ = ft.rangeQuery(null, 0.105, true, false, MOCK_QSC);
scaledFloatQ = ft.rangeQuery(null, 0.105, true, false, null);
assertEquals("scaled_float:[-9223372036854775808 TO 10]", scaledFloatQ.toString());
scaledFloatQ = ft.rangeQuery(null, 0.105, true, true, MOCK_QSC);
scaledFloatQ = ft.rangeQuery(null, 0.105, true, true, null);
assertEquals("scaled_float:[-9223372036854775808 TO 10]", scaledFloatQ.toString());
scaledFloatQ = ft.rangeQuery(null, 79.99, true, true, MOCK_QSC);
scaledFloatQ = ft.rangeQuery(null, 79.99, true, true, null);
assertEquals("scaled_float:[-9223372036854775808 TO 7999]", scaledFloatQ.toString());
}

Expand All @@ -122,15 +122,15 @@ public void testRoundsLowerBoundCorrectly() {
= new ScaledFloatFieldMapper.ScaledFloatFieldType("scaled_float", 100);
Query scaledFloatQ = ft.rangeQuery(-0.1, null, false, true, MOCK_QSC);
assertEquals("scaled_float:[-9 TO 9223372036854775807]", scaledFloatQ.toString());
scaledFloatQ = ft.rangeQuery(-0.1, null, true, true, MOCK_QSC);
scaledFloatQ = ft.rangeQuery(-0.1, null, true, true, null);
assertEquals("scaled_float:[-10 TO 9223372036854775807]", scaledFloatQ.toString());
scaledFloatQ = ft.rangeQuery(-0.095, null, false, true, MOCK_QSC);
scaledFloatQ = ft.rangeQuery(-0.095, null, false, true, null);
assertEquals("scaled_float:[-9 TO 9223372036854775807]", scaledFloatQ.toString());
scaledFloatQ = ft.rangeQuery(-0.095, null, true, true, MOCK_QSC);
scaledFloatQ = ft.rangeQuery(-0.095, null, true, true, null);
assertEquals("scaled_float:[-9 TO 9223372036854775807]", scaledFloatQ.toString());
scaledFloatQ = ft.rangeQuery(-0.105, null, false, true, MOCK_QSC);
scaledFloatQ = ft.rangeQuery(-0.105, null, false, true, null);
assertEquals("scaled_float:[-10 TO 9223372036854775807]", scaledFloatQ.toString());
scaledFloatQ = ft.rangeQuery(-0.105, null, true, true, MOCK_QSC);
scaledFloatQ = ft.rangeQuery(-0.105, null, true, true, null);
assertEquals("scaled_float:[-10 TO 9223372036854775807]", scaledFloatQ.toString());
}

Expand Down
24 changes: 9 additions & 15 deletions modules/percolator/src/test/java/org/elasticsearch/percolator/CandidateQueryTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.NumberFieldMapper;
import org.elasticsearch.index.mapper.ParseContext;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESSingleNodeTestCase;
import org.junit.After;
Expand Down Expand Up @@ -124,7 +123,6 @@ public class CandidateQueryTests extends ESSingleNodeTestCase {
private IndexWriter indexWriter;
private DocumentMapper documentMapper;
private DirectoryReader directoryReader;
private IndexService indexService;
private MapperService mapperService;

private PercolatorFieldMapper fieldMapper;
Expand All @@ -146,7 +144,7 @@ public void init() throws Exception {
indexWriter = new IndexWriter(directory, config);

String indexName = "test";
indexService = createIndex(indexName, Settings.EMPTY);
IndexService indexService = createIndex(indexName, Settings.EMPTY);
mapperService = indexService.mapperService();

String mapper = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
Expand Down Expand Up @@ -199,7 +197,6 @@ public void testDuel() throws Exception {
}
Collections.sort(intValues);

QueryShardContext context = createSearchContext(indexService).getQueryShardContext();
MappedFieldType intFieldType = mapperService.fieldType("int_field");

List<Supplier<Query>> queryFunctions = new ArrayList<>();
Expand All @@ -210,10 +207,10 @@ public void testDuel() throws Exception {
queryFunctions.add(() -> new TermQuery(new Term(field1, randomFrom(stringContent.get(field1)))));
String field2 = randomFrom(stringFields);
queryFunctions.add(() -> new TermQuery(new Term(field2, randomFrom(stringContent.get(field2)))));
queryFunctions.add(() -> intFieldType.termQuery(randomFrom(intValues), context));
queryFunctions.add(() -> intFieldType.termsQuery(Arrays.asList(randomFrom(intValues), randomFrom(intValues)), context));
queryFunctions.add(() -> intFieldType.termQuery(randomFrom(intValues), null));
queryFunctions.add(() -> intFieldType.termsQuery(Arrays.asList(randomFrom(intValues), randomFrom(intValues)), null));
queryFunctions.add(() -> intFieldType.rangeQuery(intValues.get(4), intValues.get(intValues.size() - 4), true,
true, ShapeRelation.WITHIN, null, null, context));
true, ShapeRelation.WITHIN, null, null, null));
queryFunctions.add(() -> new TermInSetQuery(field1, new BytesRef(randomFrom(stringContent.get(field1))),
new BytesRef(randomFrom(stringContent.get(field1)))));
queryFunctions.add(() -> new TermInSetQuery(field2, new BytesRef(randomFrom(stringContent.get(field1))),
Expand Down Expand Up @@ -338,7 +335,6 @@ public void testDuel2() throws Exception {
ranges.add(new int[]{0, 10});
ranges.add(new int[]{15, 50});

QueryShardContext context = createSearchContext(indexService).getQueryShardContext();
List<ParseContext.Document> documents = new ArrayList<>();
{
addQuery(new TermQuery(new Term("string_field", randomFrom(stringValues))), documents);
Expand All @@ -348,13 +344,13 @@ public void testDuel2() throws Exception {
}
{
int[] range = randomFrom(ranges);
Query rangeQuery = intFieldType.rangeQuery(range[0], range[1], true, true, null, null, null, context);
Query rangeQuery = intFieldType.rangeQuery(range[0], range[1], true, true, null, null, null, null);
addQuery(rangeQuery, documents);
}
{
int numBooleanQueries = randomIntBetween(1, 5);
for (int i = 0; i < numBooleanQueries; i++) {
Query randomBQ = randomBQ(1, stringValues, ranges, intFieldType, context);
Query randomBQ = randomBQ(1, stringValues, ranges, intFieldType);
addQuery(randomBQ, documents);
}
}
Expand All @@ -379,7 +375,6 @@ public void testDuel2() throws Exception {
MemoryIndex memoryIndex = MemoryIndex.fromDocument(document, new WhitespaceAnalyzer());
duelRun(queryStore, memoryIndex, shardSearcher);
}

for (int[] range : ranges) {
List<Field> numberFields =
NumberFieldMapper.NumberType.INTEGER.createFields("int_field", between(range[0], range[1]), true, true, false);
Expand All @@ -392,8 +387,7 @@ public void testDuel2() throws Exception {
}
}

private BooleanQuery randomBQ(int depth, List<String> stringValues, List<int[]> ranges,
MappedFieldType intFieldType, QueryShardContext context) {
private BooleanQuery randomBQ(int depth, List<String> stringValues, List<int[]> ranges, MappedFieldType intFieldType) {
final int numClauses = randomIntBetween(1, 4);
final boolean onlyShouldClauses = randomBoolean();
final BooleanQuery.Builder builder = new BooleanQuery.Builder();
Expand All @@ -402,10 +396,10 @@ private BooleanQuery randomBQ(int depth, List<String> stringValues, List<int[]>
for (int i = 0; i < numClauses; i++) {
Query subQuery;
if (randomBoolean() && depth <= 3) {
subQuery = randomBQ(depth + 1, stringValues, ranges, intFieldType, context);
subQuery = randomBQ(depth + 1, stringValues, ranges, intFieldType);
} else if (randomBoolean()) {
int[] range = randomFrom(ranges);
subQuery = intFieldType.rangeQuery(range[0], range[1], true, true, null, null, null, context);
subQuery = intFieldType.rangeQuery(range[0], range[1], true, true, null, null, null, null);
} else {
subQuery = new TermQuery(new Term("string_field", randomFrom(stringValues)));
}
Expand Down
7 changes: 3 additions & 4 deletions ...les/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,14 +227,13 @@ public void testExtractTerms() throws Exception {
}

public void testExtractRanges() throws Exception {
QueryShardContext context = createSearchContext(indexService).getQueryShardContext();
addQueryFieldMappings();
BooleanQuery.Builder bq = new BooleanQuery.Builder();
Query rangeQuery1 = mapperService.fieldType("number_field1")
.rangeQuery(10, 20, true, true, null, null, null, context);
.rangeQuery(10, 20, true, true, null, null, null, null);
bq.add(rangeQuery1, Occur.MUST);
Query rangeQuery2 = mapperService.fieldType("number_field1")
.rangeQuery(15, 20, true, true, null, null, null, context);
.rangeQuery(15, 20, true, true, null, null, null, null);
bq.add(rangeQuery2, Occur.MUST);

DocumentMapper documentMapper = mapperService.documentMapper("doc");
Expand Down Expand Up @@ -266,7 +265,7 @@ public void testExtractRanges() throws Exception {
bq = new BooleanQuery.Builder();
bq.add(rangeQuery1, Occur.MUST);
rangeQuery2 = mapperService.fieldType("number_field2")
.rangeQuery(15, 20, true, true, null, null, null, context);
.rangeQuery(15, 20, true, true, null, null, null, null);
bq.add(rangeQuery2, Occur.MUST);

parseContext = new ParseContext.InternalParseContext(settings,
Expand Down
5 changes: 0 additions & 5 deletions server/src/main/java/org/elasticsearch/index/IndexSortConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,6 @@ public boolean hasIndexSort() {
return sortSpecs.length > 0;
}

public boolean hasPrimarySortOnField(String field) {
return sortSpecs.length > 0
&& sortSpecs[0].field.equals(field);
}

/**
* Builds the {@link Sort} order from the settings for this index
* or returns null if this index has no sort.
Expand Down
7 changes: 0 additions & 7 deletions server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.apache.lucene.search.BoostQuery;
import org.apache.lucene.search.DocValuesFieldExistsQuery;
import org.apache.lucene.search.IndexOrDocValuesQuery;
import org.apache.lucene.search.IndexSortSortedNumericDocValuesRangeQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.util.BytesRef;
Expand Down Expand Up @@ -390,17 +389,11 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
--u;
}
}

Query query = LongPoint.newRangeQuery(name(), l, u);
if (hasDocValues()) {
Query dvQuery = SortedNumericDocValuesField.newSlowRangeQuery(name(), l, u);
query = new IndexOrDocValuesQuery(query, dvQuery);

if (context.indexSortedOnField(name())) {
query = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, query);
}
}

if (nowUsed[0]) {
query = new DateRangeIncludingNowQuery(query);
}
Expand Down
Loading

0 comments on commit 13b9536

Please sign in to comment.