Query enhancement: single value numeric queries shouldn't be handled by NumericRangeQuery and should use a TermQuery wrapped in a ConstantScoreQuery instead.

Equally, single value filters should use TermFilters rather than NumericRangeFilters

Closes #10646
This commit is contained in:
markharwood 2015-04-16 16:02:53 +01:00
parent a60571c597
commit 05c3d05cff
11 changed files with 48 additions and 145 deletions

View File

@ -200,13 +200,6 @@ public class ByteFieldMapper extends NumberFieldMapper<Byte> {
true, true); 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 @Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) { public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeQuery.newIntRange(names.indexName(), precisionStep, return NumericRangeQuery.newIntRange(names.indexName(), precisionStep,
@ -216,14 +209,8 @@ public class ByteFieldMapper extends NumberFieldMapper<Byte> {
} }
@Override @Override
public Filter termFilter(Object value, @Nullable QueryParseContext context) { public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper,
int iValue = parseValueAsInt(value); @Nullable QueryParseContext context) {
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, return Queries.wrap(NumericRangeQuery.newIntRange(names.indexName(), precisionStep,
lowerTerm == null ? null : parseValueAsInt(lowerTerm), lowerTerm == null ? null : parseValueAsInt(lowerTerm),
upperTerm == null ? null : parseValueAsInt(upperTerm), upperTerm == null ? null : parseValueAsInt(upperTerm),

View File

@ -297,13 +297,6 @@ public class DateFieldMapper extends NumberFieldMapper<Long> {
true, true); 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) { public long parseToMilliseconds(Object value) {
return parseToMilliseconds(value, false, null, dateMathParser); return parseToMilliseconds(value, false, null, dateMathParser);
} }
@ -323,13 +316,6 @@ public class DateFieldMapper extends NumberFieldMapper<Long> {
return dateParser.parse(value, now(), inclusive, zone); 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 @Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) { public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return rangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, null, null, context); return rangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, null, null, context);

View File

@ -191,13 +191,6 @@ public class DoubleFieldMapper extends NumberFieldMapper<Double> {
true, true); 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 @Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) { public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeQuery.newDoubleRange(names.indexName(), precisionStep, return NumericRangeQuery.newDoubleRange(names.indexName(), precisionStep,
@ -206,13 +199,6 @@ public class DoubleFieldMapper extends NumberFieldMapper<Double> {
includeLower, includeUpper); 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 @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.newDoubleRange(names.indexName(), precisionStep, return Queries.wrap(NumericRangeQuery.newDoubleRange(names.indexName(), precisionStep,

View File

@ -201,13 +201,6 @@ public class FloatFieldMapper extends NumberFieldMapper<Float> {
true, true); 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 @Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) { public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeQuery.newFloatRange(names.indexName(), precisionStep, return NumericRangeQuery.newFloatRange(names.indexName(), precisionStep,
@ -216,13 +209,6 @@ public class FloatFieldMapper extends NumberFieldMapper<Float> {
includeLower, includeUpper); 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 @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.newFloatRange(names.indexName(), precisionStep, return Queries.wrap(NumericRangeQuery.newFloatRange(names.indexName(), precisionStep,

View File

@ -195,20 +195,6 @@ public class IntegerFieldMapper extends NumberFieldMapper<Integer> {
true, true); 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 @Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) { public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeQuery.newIntRange(names.indexName(), precisionStep, return NumericRangeQuery.newIntRange(names.indexName(), precisionStep,

View File

@ -185,20 +185,6 @@ public class LongFieldMapper extends NumberFieldMapper<Long> {
true, true); 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 @Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) { public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeQuery.newLongRange(names.indexName(), precisionStep, return NumericRangeQuery.newLongRange(names.indexName(), precisionStep,

View File

@ -33,8 +33,11 @@ import org.apache.lucene.index.DocValuesType;
import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.IndexableField; import org.apache.lucene.index.IndexableField;
import org.apache.lucene.index.IndexableFieldType; 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.Filter;
import org.apache.lucene.search.Query; import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.store.ByteArrayDataOutput; import org.apache.lucene.store.ByteArrayDataOutput;
import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.BytesRef;
import org.elasticsearch.ElasticsearchIllegalArgumentException; import org.elasticsearch.ElasticsearchIllegalArgumentException;
@ -271,22 +274,18 @@ public abstract class NumberFieldMapper<T extends Number> extends AbstractFieldM
return true; return true;
} }
/**
* Numeric field level query are basically range queries with same value and included. That's the recommended
* way to execute it.
*/
@Override @Override
public Query termQuery(Object value, @Nullable QueryParseContext context) { public final Query termQuery(Object value, @Nullable QueryParseContext context) {
return rangeQuery(value, value, true, true, 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 @Override
public Filter termFilter(Object value, @Nullable QueryParseContext context) { public final Filter termFilter(Object value, @Nullable QueryParseContext context) {
return rangeFilter(value, value, true, true, 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 @Override

View File

@ -201,13 +201,6 @@ public class ShortFieldMapper extends NumberFieldMapper<Short> {
true, true); 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 @Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) { public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeQuery.newIntRange(names.indexName(), precisionStep, return NumericRangeQuery.newIntRange(names.indexName(), precisionStep,
@ -216,13 +209,6 @@ public class ShortFieldMapper extends NumberFieldMapper<Short> {
includeLower, includeUpper); 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 @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, return Queries.wrap(NumericRangeQuery.newIntRange(names.indexName(), precisionStep,

View File

@ -464,25 +464,25 @@ public class SimpleIndexQueryParserTests extends ElasticsearchSingleNodeTest {
public void testTermQueryBuilder() throws IOException { public void testTermQueryBuilder() throws IOException {
IndexQueryParserService queryParser = queryParser(); IndexQueryParserService queryParser = queryParser();
Query parsedQuery = queryParser.parse(termQuery("age", 34).buildAsBytes()).query(); Query parsedQuery = queryParser.parse(termQuery("age", 34).buildAsBytes()).query();
assertThat(parsedQuery, instanceOf(NumericRangeQuery.class)); TermQuery fieldQuery = unwrapTermQuery(parsedQuery, true);
NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery; assertThat(fieldQuery.getTerm().bytes(), equalTo(indexedValueForSearch(34l)));
assertThat(fieldQuery.getMin().intValue(), equalTo(34));
assertThat(fieldQuery.getMax().intValue(), equalTo(34));
assertThat(fieldQuery.includesMax(), equalTo(true));
assertThat(fieldQuery.includesMin(), equalTo(true));
} }
@Test @Test
public void testTermQuery() throws IOException { public void testTermQuery() throws IOException {
IndexQueryParserService queryParser = queryParser(); IndexQueryParserService queryParser = queryParser();
String query = copyToStringFromClasspath("/org/elasticsearch/index/query/term.json"); String query = copyToStringFromClasspath("/org/elasticsearch/index/query/term.json");
Query parsedQuery = queryParser.parse(query).query(); TermQuery fieldQuery = unwrapTermQuery(queryParser.parse(query).query(), true);
assertThat(parsedQuery, instanceOf(NumericRangeQuery.class)); assertThat(fieldQuery.getTerm().bytes(), equalTo(indexedValueForSearch(34l)));
NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery; }
assertThat(fieldQuery.getMin().intValue(), equalTo(34));
assertThat(fieldQuery.getMax().intValue(), equalTo(34)); private static TermQuery unwrapTermQuery(Query q, boolean expectConstantWrapper) {
assertThat(fieldQuery.includesMax(), equalTo(true)); if (expectConstantWrapper) {
assertThat(fieldQuery.includesMin(), equalTo(true)); assertThat(q, instanceOf(ConstantScoreQuery.class));
q = ((ConstantScoreQuery) q).getQuery();
}
assertThat(q, instanceOf(TermQuery.class));
return (TermQuery) q;
} }
@Test @Test
@ -543,14 +543,19 @@ public class SimpleIndexQueryParserTests extends ElasticsearchSingleNodeTest {
@Test @Test
public void testTermWithBoostQueryBuilder() throws IOException { public void testTermWithBoostQueryBuilder() throws IOException {
IndexQueryParserService queryParser = queryParser(); IndexQueryParserService queryParser = queryParser();
Query parsedQuery = queryParser.parse(termQuery("age", 34).boost(2.0f)).query(); Query parsedQuery = queryParser.parse(termQuery("age", 34).boost(2.0f)).query();
assertThat(parsedQuery, instanceOf(NumericRangeQuery.class)); TermQuery fieldQuery = unwrapTermQuery(parsedQuery, true);
NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery; assertThat(fieldQuery.getTerm().bytes(), equalTo(indexedValueForSearch(34l)));
assertThat(fieldQuery.getMin().intValue(), equalTo(34)); assertThat((double) parsedQuery.getBoost(), closeTo(2.0, 0.01));
assertThat(fieldQuery.getMax().intValue(), equalTo(34)); }
assertThat(fieldQuery.includesMax(), equalTo(true));
assertThat(fieldQuery.includesMin(), equalTo(true)); private BytesRef indexedValueForSearch(long value) {
assertThat((double) fieldQuery.getBoost(), closeTo(2.0, 0.01)); BytesRefBuilder bytesRef = new BytesRefBuilder();
NumericUtils.longToPrefixCoded(value, 0, bytesRef); // 0 because of
// exact
// match
return bytesRef.get();
} }
@Test @Test
@ -558,13 +563,9 @@ public class SimpleIndexQueryParserTests extends ElasticsearchSingleNodeTest {
IndexQueryParserService queryParser = queryParser(); IndexQueryParserService queryParser = queryParser();
String query = copyToStringFromClasspath("/org/elasticsearch/index/query/term-with-boost.json"); String query = copyToStringFromClasspath("/org/elasticsearch/index/query/term-with-boost.json");
Query parsedQuery = queryParser.parse(query).query(); Query parsedQuery = queryParser.parse(query).query();
assertThat(parsedQuery, instanceOf(NumericRangeQuery.class)); TermQuery fieldQuery = unwrapTermQuery(parsedQuery, true);
NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery; assertThat(fieldQuery.getTerm().bytes(), equalTo(indexedValueForSearch(34l)));
assertThat(fieldQuery.getMin().intValue(), equalTo(34)); assertThat((double) parsedQuery.getBoost(), closeTo(2.0, 0.01));
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));
} }
@Test @Test

View File

@ -18,8 +18,6 @@
*/ */
package org.elasticsearch.percolator; package org.elasticsearch.percolator;
import com.google.common.base.Predicate;
import org.elasticsearch.action.ShardOperationFailedException; import org.elasticsearch.action.ShardOperationFailedException;
import org.elasticsearch.action.admin.cluster.node.stats.NodeStats; import org.elasticsearch.action.admin.cluster.node.stats.NodeStats;
import org.elasticsearch.action.admin.cluster.node.stats.NodesStatsResponse; import org.elasticsearch.action.admin.cluster.node.stats.NodesStatsResponse;
@ -32,9 +30,9 @@ import org.elasticsearch.action.percolate.PercolateResponse;
import org.elasticsearch.action.percolate.PercolateSourceBuilder; import org.elasticsearch.action.percolate.PercolateSourceBuilder;
import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.client.Client;
import org.elasticsearch.client.Requests; import org.elasticsearch.client.Requests;
import org.elasticsearch.common.Strings; 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;
import org.elasticsearch.common.settings.ImmutableSettings.Builder; import org.elasticsearch.common.settings.ImmutableSettings.Builder;
import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.unit.TimeValue;
@ -1329,7 +1327,9 @@ public class PercolatorTests extends ElasticsearchIntegrationTest {
.setSortByScore(true) .setSortByScore(true)
.setSize(size) .setSize(size)
.setPercolateDoc(docBuilder().setDoc("field", "value")) .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(); .execute().actionGet();
assertMatchCount(response, levels.size()); assertMatchCount(response, levels.size());

View File

@ -25,8 +25,6 @@ import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryRespon
import org.elasticsearch.client.Client; import org.elasticsearch.client.Client;
import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.ImmutableSettings; 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.unit.Fuzziness;
import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.query.FilterBuilders; import org.elasticsearch.index.query.FilterBuilders;
@ -48,7 +46,9 @@ import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF
import static org.elasticsearch.index.query.QueryBuilders.queryStringQuery; import static org.elasticsearch.index.query.QueryBuilders.queryStringQuery;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; 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;
/** /**
* *