Skip to content

Commit

Permalink
Query enhancement: Single value numeric queries shouldn’t be handled …
Browse files Browse the repository at this point in the history 
…by NumericRangeQuery.

They should use a ConstantScoreQuery wrapping a TermQuery as this is more efficient.
Similarly, single value numeric filters shouldn’t use a NumericRangeFilter

Closes #10646
  • Loading branch information
@markharwood
markharwood committed Apr 22, 2015
1 parent 75e124e commit f9e044d
Show file tree
Hide file tree
Showing 11 changed files with 98 additions and 147 deletions.
14 changes: 0 additions & 14 deletions src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java
View file
Expand Up @@ -195,13 +195,6 @@ public Query fuzzyQuery(String value, Fuzziness fuzziness, int prefixLength, int
true, true);
}

@Override
public Query termQuery(Object value, @Nullable QueryParseContext context) {
int iValue = parseValue(value);
return NumericRangeQuery.newIntRange(names.indexName(), precisionStep,
iValue, iValue, true, true);
}

@Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeQuery.newIntRange(names.indexName(), precisionStep,
Expand All @@ -210,13 +203,6 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
includeLower, includeUpper);
}

@Override
public Filter termFilter(Object value, @Nullable QueryParseContext context) {
int iValue = parseValueAsInt(value);
return NumericRangeFilter.newIntRange(names.indexName(), precisionStep,
iValue, iValue, true, true);
}

@Override
public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeFilter.newIntRange(names.indexName(), precisionStep,
Expand Down
14 changes: 0 additions & 14 deletions src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java
View file
Expand Up @@ -309,13 +309,6 @@ public Query fuzzyQuery(String value, Fuzziness fuzziness, int prefixLength, int
true, true);
}

@Override
public Query termQuery(Object value, @Nullable QueryParseContext context) {
long lValue = parseToMilliseconds(value);
return NumericRangeQuery.newLongRange(names.indexName(), precisionStep,
lValue, lValue, true, true);
}

public long parseToMilliseconds(Object value) {
return parseToMilliseconds(value, false, null, dateMathParser);
}
Expand All @@ -336,13 +329,6 @@ public long parseToMilliseconds(String value, boolean inclusive, @Nullable DateT
return dateParser.parse(value, now(), roundUp, zone);
}

@Override
public Filter termFilter(Object value, @Nullable QueryParseContext context) {
final long lValue = parseToMilliseconds(value);
return NumericRangeFilter.newLongRange(names.indexName(), precisionStep,
lValue, lValue, true, true);
}

@Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return rangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, null, null, context);
Expand Down
14 changes: 0 additions & 14 deletions src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java
View file
Expand Up @@ -186,13 +186,6 @@ public Query fuzzyQuery(String value, Fuzziness fuzziness, int prefixLength, int
true, true);
}

@Override
public Query termQuery(Object value, @Nullable QueryParseContext context) {
double dValue = parseDoubleValue(value);
return NumericRangeQuery.newDoubleRange(names.indexName(), precisionStep,
dValue, dValue, true, true);
}

@Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeQuery.newDoubleRange(names.indexName(), precisionStep,
Expand All @@ -201,13 +194,6 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
includeLower, includeUpper);
}

@Override
public Filter termFilter(Object value, @Nullable QueryParseContext context) {
double dValue = parseDoubleValue(value);
return NumericRangeFilter.newDoubleRange(names.indexName(), precisionStep,
dValue, dValue, true, true);
}

@Override
public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeFilter.newDoubleRange(names.indexName(), precisionStep,
Expand Down
14 changes: 0 additions & 14 deletions src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java
View file
Expand Up @@ -195,13 +195,6 @@ public Query fuzzyQuery(String value, Fuzziness fuzziness, int prefixLength, int
true, true);
}

@Override
public Query termQuery(Object value, @Nullable QueryParseContext context) {
float fValue = parseValue(value);
return NumericRangeQuery.newFloatRange(names.indexName(), precisionStep,
fValue, fValue, true, true);
}

@Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeQuery.newFloatRange(names.indexName(), precisionStep,
Expand All @@ -210,13 +203,6 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
includeLower, includeUpper);
}

@Override
public Filter termFilter(Object value, @Nullable QueryParseContext context) {
float fValue = parseValue(value);
return NumericRangeFilter.newFloatRange(names.indexName(), precisionStep,
fValue, fValue, true, true);
}

@Override
public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeFilter.newFloatRange(names.indexName(), precisionStep,
Expand Down
14 changes: 0 additions & 14 deletions src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java
View file
Expand Up @@ -190,20 +190,6 @@ public Query fuzzyQuery(String value, Fuzziness fuzziness, int prefixLength, int
true, true);
}

@Override
public Query termQuery(Object value, @Nullable QueryParseContext context) {
int iValue = parseValue(value);
return NumericRangeQuery.newIntRange(names.indexName(), precisionStep,
iValue, iValue, true, true);
}

@Override
public Filter termFilter(Object value, @Nullable QueryParseContext context) {
int iValue = parseValue(value);
return NumericRangeFilter.newIntRange(names.indexName(), precisionStep,
iValue, iValue, true, true);
}

@Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeQuery.newIntRange(names.indexName(), precisionStep,
Expand Down
14 changes: 0 additions & 14 deletions src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java
View file
Expand Up @@ -180,20 +180,6 @@ public Query fuzzyQuery(String value, Fuzziness fuzziness, int prefixLength, int
true, true);
}

@Override
public Query termQuery(Object value, @Nullable QueryParseContext context) {
long iValue = parseLongValue(value);
return NumericRangeQuery.newLongRange(names.indexName(), precisionStep,
iValue, iValue, true, true);
}

@Override
public Filter termFilter(Object value, @Nullable QueryParseContext context) {
long iValue = parseLongValue(value);
return NumericRangeFilter.newLongRange(names.indexName(), precisionStep,
iValue, iValue, true, true);
}

@Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeQuery.newLongRange(names.indexName(), precisionStep,
Expand Down
23 changes: 11 additions & 12 deletions src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java
View file
Expand Up @@ -32,8 +32,11 @@
import org.apache.lucene.index.FieldInfo.IndexOptions;
import org.apache.lucene.index.IndexableField;
import org.apache.lucene.index.IndexableFieldType;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.Filter;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.store.ByteArrayDataOutput;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.ElasticsearchIllegalArgumentException;
Expand Down Expand Up @@ -269,22 +272,18 @@ public boolean useTermQueryWithQueryString() {
return true;
}

/**
* Numeric field level query are basically range queries with same value and included. That's the recommended
* way to execute it.
*/
@Override
public Query termQuery(Object value, @Nullable QueryParseContext context) {
return rangeQuery(value, value, true, true, context);
public final Query termQuery(Object value, @Nullable QueryParseContext context) {
TermQuery scoringQuery = new TermQuery(new Term(names.indexName(), indexedValueForSearch(value)));
return new ConstantScoreQuery(scoringQuery);
}

/**
* Numeric field level filter are basically range queries with same value and included. That's the recommended
* way to execute it.
*/
@Override
public Filter termFilter(Object value, @Nullable QueryParseContext context) {
return rangeFilter(value, value, true, true, context);
public final Filter termFilter(Object value, @Nullable QueryParseContext context) {
// Made this method final because previously many subclasses duplicated
// the same code, returning a NumericRangeFilter, which should be less
// efficient than super's default impl of a single TermFilter.
return super.termFilter(value, context);
}

@Override
Expand Down
14 changes: 0 additions & 14 deletions src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java
View file
Expand Up @@ -196,13 +196,6 @@ public Query fuzzyQuery(String value, Fuzziness fuzziness, int prefixLength, int
true, true);
}

@Override
public Query termQuery(Object value, @Nullable QueryParseContext context) {
int iValue = parseValueAsInt(value);
return NumericRangeQuery.newIntRange(names.indexName(), precisionStep,
iValue, iValue, true, true);
}

@Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeQuery.newIntRange(names.indexName(), precisionStep,
Expand All @@ -211,13 +204,6 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
includeLower, includeUpper);
}

@Override
public Filter termFilter(Object value, @Nullable QueryParseContext context) {
int iValue = parseValueAsInt(value);
return NumericRangeFilter.newIntRange(names.indexName(), precisionStep,
iValue, iValue, true, true);
}

@Override
public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeFilter.newIntRange(names.indexName(), precisionStep,
Expand Down
55 changes: 28 additions & 27 deletions src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java
View file
Expand Up @@ -400,25 +400,25 @@ public void testDisMax2() throws Exception {
public void testTermQueryBuilder() throws IOException {
IndexQueryParserService queryParser = queryParser();
Query parsedQuery = queryParser.parse(termQuery("age", 34).buildAsBytes()).query();
assertThat(parsedQuery, instanceOf(NumericRangeQuery.class));
NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery;
assertThat(fieldQuery.getMin().intValue(), equalTo(34));
assertThat(fieldQuery.getMax().intValue(), equalTo(34));
assertThat(fieldQuery.includesMax(), equalTo(true));
assertThat(fieldQuery.includesMin(), equalTo(true));
TermQuery fieldQuery = unwrapTermQuery(parsedQuery, true);
assertThat(fieldQuery.getTerm().bytes(), equalTo(indexedValueForSearch(34l)));
}

@Test
public void testTermQuery() throws IOException {
IndexQueryParserService queryParser = queryParser();
String query = copyToStringFromClasspath("/org/elasticsearch/index/query/term.json");
Query parsedQuery = queryParser.parse(query).query();
assertThat(parsedQuery, instanceOf(NumericRangeQuery.class));
NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery;
assertThat(fieldQuery.getMin().intValue(), equalTo(34));
assertThat(fieldQuery.getMax().intValue(), equalTo(34));
assertThat(fieldQuery.includesMax(), equalTo(true));
assertThat(fieldQuery.includesMin(), equalTo(true));
TermQuery fieldQuery = unwrapTermQuery(queryParser.parse(query).query(), true);
assertThat(fieldQuery.getTerm().bytes(), equalTo(indexedValueForSearch(34l)));
}

private static TermQuery unwrapTermQuery(Query q, boolean expectConstantWrapper) {
if (expectConstantWrapper) {
assertThat(q, instanceOf(ConstantScoreQuery.class));
q = ((ConstantScoreQuery) q).getQuery();
}
assertThat(q, instanceOf(TermQuery.class));
return (TermQuery) q;
}

@Test
Expand Down Expand Up @@ -479,28 +479,29 @@ public void testFuzzyQueryWithFields2() throws IOException {
@Test
public void testTermWithBoostQueryBuilder() throws IOException {
IndexQueryParserService queryParser = queryParser();

Query parsedQuery = queryParser.parse(termQuery("age", 34).boost(2.0f)).query();
assertThat(parsedQuery, instanceOf(NumericRangeQuery.class));
NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery;
assertThat(fieldQuery.getMin().intValue(), equalTo(34));
assertThat(fieldQuery.getMax().intValue(), equalTo(34));
assertThat(fieldQuery.includesMax(), equalTo(true));
assertThat(fieldQuery.includesMin(), equalTo(true));
assertThat((double) fieldQuery.getBoost(), closeTo(2.0, 0.01));
TermQuery fieldQuery = unwrapTermQuery(parsedQuery, true);
assertThat(fieldQuery.getTerm().bytes(), equalTo(indexedValueForSearch(34l)));
assertThat((double) parsedQuery.getBoost(), closeTo(2.0, 0.01));
}

private BytesRef indexedValueForSearch(long value) {
BytesRefBuilder bytesRef = new BytesRefBuilder();
NumericUtils.longToPrefixCoded(value, 0, bytesRef); // 0 because of
// exact
// match
return bytesRef.get();
}

@Test
public void testTermWithBoostQuery() throws IOException {
IndexQueryParserService queryParser = queryParser();
String query = copyToStringFromClasspath("/org/elasticsearch/index/query/term-with-boost.json");
Query parsedQuery = queryParser.parse(query).query();
assertThat(parsedQuery, instanceOf(NumericRangeQuery.class));
NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery;
assertThat(fieldQuery.getMin().intValue(), equalTo(34));
assertThat(fieldQuery.getMax().intValue(), equalTo(34));
assertThat(fieldQuery.includesMax(), equalTo(true));
assertThat(fieldQuery.includesMin(), equalTo(true));
assertThat((double) fieldQuery.getBoost(), closeTo(2.0, 0.01));
TermQuery fieldQuery = unwrapTermQuery(parsedQuery, true);
assertThat(fieldQuery.getTerm().bytes(), equalTo(indexedValueForSearch(34l)));
assertThat((double) parsedQuery.getBoost(), closeTo(2.0, 0.01));
}

@Test
Expand Down
48 changes: 42 additions & 6 deletions src/test/java/org/elasticsearch/percolator/PercolatorTests.java
View file
Expand Up @@ -34,6 +34,7 @@
import org.elasticsearch.client.Client;
import org.elasticsearch.client.Requests;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.lucene.search.function.CombineFunction;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.ImmutableSettings.Builder;
import org.elasticsearch.common.unit.TimeValue;
Expand All @@ -54,18 +55,51 @@
import org.junit.Test;

import java.io.IOException;
import java.util.*;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.NavigableSet;
import java.util.Set;
import java.util.TreeSet;

import static org.elasticsearch.action.percolate.PercolateSourceBuilder.docBuilder;
import static org.elasticsearch.common.settings.ImmutableSettings.builder;
import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
import static org.elasticsearch.common.xcontent.XContentFactory.*;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.common.xcontent.XContentFactory.smileBuilder;
import static org.elasticsearch.common.xcontent.XContentFactory.yamlBuilder;
import static org.elasticsearch.index.query.FilterBuilders.rangeFilter;
import static org.elasticsearch.index.query.FilterBuilders.termFilter;
import static org.elasticsearch.index.query.QueryBuilders.*;
import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
import static org.elasticsearch.index.query.QueryBuilders.constantScoreQuery;
import static org.elasticsearch.index.query.QueryBuilders.functionScoreQuery;
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
import static org.elasticsearch.index.query.QueryBuilders.matchQuery;
import static org.elasticsearch.index.query.QueryBuilders.rangeQuery;
import static org.elasticsearch.index.query.QueryBuilders.termQuery;
import static org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders.scriptFunction;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*;
import static org.hamcrest.Matchers.*;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAllSuccessful;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertMatchCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
import static org.hamcrest.Matchers.arrayWithSize;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.emptyArray;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;

/**
*
Expand Down Expand Up @@ -1290,7 +1324,9 @@ public void testPercolateScoreAndSorting() throws Exception {
.setSortByScore(true)
.setSize(size)
.setPercolateDoc(docBuilder().setDoc("field", "value"))
.setPercolateQuery(QueryBuilders.functionScoreQuery(matchQuery("field1", value), scriptFunction("doc['level'].value")))
.setPercolateQuery(
QueryBuilders.functionScoreQuery(matchQuery("field1", value), scriptFunction("doc['level'].value")).boostMode(
CombineFunction.REPLACE))
.execute().actionGet();

assertMatchCount(response, levels.size());
Expand Down

0 comments on commit f9e044d

Please sign in to comment.