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 and should use a TermQuery wrapped in a ConstantScoreQuery instead.

Equally, single value filters should use TermFilters rather than NumericRangeFilters

Closes #10646
  • Loading branch information
@markharwood
markharwood committed Apr 22, 2015
1 parent a60571c commit 05c3d05
Show file tree
Hide file tree
Showing 11 changed files with 48 additions and 145 deletions.
17 changes: 2 additions & 15 deletions src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java
View file
Expand Up @@ -200,13 +200,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 @@ -216,14 +209,8 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
}

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

@Override
public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper,
@Nullable QueryParseContext context) {
return Queries.wrap(NumericRangeQuery.newIntRange(names.indexName(), precisionStep,
lowerTerm == null ? null : parseValueAsInt(lowerTerm),
upperTerm == null ? null : parseValueAsInt(upperTerm),
Expand Down
14 changes: 0 additions & 14 deletions src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java
View file
Expand Up @@ -297,13 +297,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 @@ -323,13 +316,6 @@ public long parseToMilliseconds(String value, boolean inclusive, @Nullable DateT
return dateParser.parse(value, now(), inclusive, zone);
}

@Override
public Filter termFilter(Object value, @Nullable QueryParseContext context) {
final long lValue = parseToMilliseconds(value);
return Queries.wrap(NumericRangeQuery.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 @@ -191,13 +191,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 @@ -206,13 +199,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 Queries.wrap(NumericRangeQuery.newDoubleRange(names.indexName(), precisionStep,
dValue, dValue, true, true));
}

@Override
public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return Queries.wrap(NumericRangeQuery.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 @@ -201,13 +201,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 @@ -216,13 +209,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 Queries.wrap(NumericRangeQuery.newFloatRange(names.indexName(), precisionStep,
fValue, fValue, true, true));
}

@Override
public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return Queries.wrap(NumericRangeQuery.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 @@ -195,20 +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 Filter termFilter(Object value, @Nullable QueryParseContext context) {
int iValue = parseValue(value);
return Queries.wrap(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 Down
14 changes: 0 additions & 14 deletions src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java
View file
Expand Up @@ -185,20 +185,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 Queries.wrap(NumericRangeQuery.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 @@ -33,8 +33,11 @@
import org.apache.lucene.index.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 @@ -271,22 +274,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 @@ -201,13 +201,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 @@ -216,13 +209,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 Queries.wrap(NumericRangeQuery.newIntRange(names.indexName(), precisionStep,
iValue, iValue, true, true));
}

@Override
public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return Queries.wrap(NumericRangeQuery.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 @@ -464,25 +464,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 @@ -543,28 +543,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
8 changes: 4 additions & 4 deletions src/test/java/org/elasticsearch/percolator/PercolatorTests.java
View file
Expand Up @@ -18,8 +18,6 @@
*/
package org.elasticsearch.percolator;

import com.google.common.base.Predicate;

import org.elasticsearch.action.ShardOperationFailedException;
import org.elasticsearch.action.admin.cluster.node.stats.NodeStats;
import org.elasticsearch.action.admin.cluster.node.stats.NodesStatsResponse;
Expand All @@ -32,9 +30,9 @@
import org.elasticsearch.action.percolate.PercolateSourceBuilder;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.IndicesOptions;
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 Down Expand Up @@ -1329,7 +1327,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
6 changes: 3 additions & 3 deletions src/test/java/org/elasticsearch/validate/SimpleValidateQueryTests.java
View file
Expand Up @@ -25,8 +25,6 @@
import org.elasticsearch.client.Client;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.DistanceUnit;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.query.FilterBuilders;
Expand All @@ -48,7 +46,9 @@
import static org.elasticsearch.index.query.QueryBuilders.queryStringQuery;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
import static org.hamcrest.Matchers.*;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;

/**
*
Expand Down

0 comments on commit 05c3d05

Please sign in to comment.