Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added execution option to range filter and deprecated numeric_range filter. #4247

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion docs/reference/query-dsl/filters/range-filter.asciidoc
Expand Up @@ -32,8 +32,25 @@ The `range` filter accepts the following parameters:

deprecated[0.90.4,The `from`, `to`, `include_lower` and `include_upper` parameters have been deprecated in favour of `gt`,`gte`,`lt`,`lte`]

[float]
==== Execution

added[1.0.0.Beta2]

The `execution` option controls how the range filter internally executes. The `execution` option accepts the following values:

[horizontal]
`index`:: Uses field's inverted in order to determine of documents fall with in the range filter's from and to range
`fielddata`:: Uses field data in order to determine of documents fall with in the range filter's from and to range.

In general for small ranges the `index` execution is faster and for longer ranges the `fielddata` execution is faster.

The `fielddata` execution as the same suggests uses field data and therefor requires more memory, so make you have
sufficient memory on your nodes in order to use this execution mode. It usually makes sense to use it on fields you're
already faceting or sorting by.

[float]
==== Caching

The result of the filter is automatically cached by default. The
The result of the filter is only automatically cached by default if the `execution` is set to `index`. The
`_cache` can be set to `false` to turn it off.
Expand Up @@ -300,7 +300,10 @@ public static RangeFilterBuilder rangeFilter(String name) {
* field data cache (loading all the values for the specified field into memory)
*
* @param name The field name
* @deprecated The numeric_range filter will be removed at some point in time in favor for the range filter with
* the execution mode <code>fielddata</code>.
*/
@Deprecated
public static NumericRangeFilterBuilder numericRangeFilter(String name) {
return new NumericRangeFilterBuilder(name);
}
Expand Down
Expand Up @@ -28,8 +28,10 @@
* <p/>
* <p>Uses the field data cache (loading all the values for the specified field into memory).
*
*
* @deprecated This filter will be removed at some point in time in favor for the range filter with the execution
* mode <code>fielddata</code>.
*/
@Deprecated
public class NumericRangeFilterBuilder extends BaseFilterBuilder {

private final String name;
Expand Down
Expand Up @@ -34,6 +34,7 @@
/**
*
*/
@Deprecated
public class NumericRangeFilterParser implements FilterParser {

public static final String NAME = "numeric_range";
Expand Down
Expand Up @@ -45,6 +45,8 @@ public class RangeFilterBuilder extends BaseFilterBuilder {

private String filterName;

private String execution;

/**
* A filter that restricts search results to values that are within the given range.
*
Expand Down Expand Up @@ -351,6 +353,24 @@ public RangeFilterBuilder cacheKey(String cacheKey) {
return this;
}

/**
* Sets the execution mode that controls how the range filter is executed. Valid values are: "index" and "fielddata".
* <ol>
* <li> The <code>index</code> execution uses the field's inverted in order to determine of documents fall with in
* the range filter's from and to range.
* <li> The <code>fielddata</code> execution uses field data in order to determine of documents fall with in the
* range filter's from and to range. Since field data is an in memory data structure, you need to have
* sufficient memory on your nodes in order to use this execution mode.
* </ol>
*
* In general for small ranges the <code>index</code> execution is faster and for longer ranges the
* <code>fielddata</code> execution is faster.
*/
public RangeFilterBuilder setExecution(String execution) {
this.execution = execution;
return this;
}

@Override
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(RangeFilterParser.NAME);
Expand All @@ -371,6 +391,9 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
if (cacheKey != null) {
builder.field("_cache_key", cacheKey);
}
if (execution != null) {
builder.field("execution", execution);
}

builder.endObject();
}
Expand Down
30 changes: 27 additions & 3 deletions src/main/java/org/elasticsearch/index/query/RangeFilterParser.java
Expand Up @@ -25,7 +25,9 @@
import org.elasticsearch.common.lucene.BytesRefs;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.cache.filter.support.CacheKeyFilter;
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.core.NumberFieldMapper;

import java.io.IOException;

Expand All @@ -51,13 +53,14 @@ public String[] names() {
public Filter parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
XContentParser parser = parseContext.parser();

boolean cache = true;
Boolean cache = null;
CacheKeyFilter.Key cacheKey = null;
String fieldName = null;
Object from = null;
Object to = null;
boolean includeLower = true;
boolean includeUpper = true;
String execution = "index";

String filterName = null;
String currentFieldName = null;
Expand Down Expand Up @@ -103,25 +106,46 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
cache = parser.booleanValue();
} else if ("_cache_key".equals(currentFieldName) || "_cacheKey".equals(currentFieldName)) {
cacheKey = new CacheKeyFilter.Key(parser.text());
} else if ("execution".equals(currentFieldName)) {
execution = parser.text();
} else {
throw new QueryParsingException(parseContext.index(), "[range] filter does not support [" + currentFieldName + "]");
}
}
}

if (fieldName == null) {
throw new QueryParsingException(parseContext.index(), "No field specified for range filter");
throw new QueryParsingException(parseContext.index(), "[range] filter no field specified for range filter");
}

Filter filter = null;
MapperService.SmartNameFieldMappers smartNameFieldMappers = parseContext.smartFieldMappers(fieldName);
if (smartNameFieldMappers != null) {
if (smartNameFieldMappers.hasMapper()) {
filter = smartNameFieldMappers.mapper().rangeFilter(from, to, includeLower, includeUpper, parseContext);
if (execution.equals("index")) {
if (cache == null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cache flag might come after / before execution, so you might be setting the flag when it has been set by the user..., I would check it after the parsing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strike that, it looks good, confused by the changes view on github...

cache = true;
}
filter = smartNameFieldMappers.mapper().rangeFilter(from, to, includeLower, includeUpper, parseContext);
} else if ("fielddata".equals(execution)) {
if (cache == null) {
cache = false;
}
FieldMapper mapper = smartNameFieldMappers.mapper();
if (!(mapper instanceof NumberFieldMapper)) {
throw new QueryParsingException(parseContext.index(), "[range] filter field [" + fieldName + "] is not a numeric type");
}
filter = ((NumberFieldMapper) mapper).rangeFilter(parseContext.fieldData(), from, to, includeLower, includeUpper, parseContext);
} else {
throw new QueryParsingException(parseContext.index(), "[range] filter doesn't support [" + execution + "] execution");
}
}
}

if (filter == null) {
if (cache == null) {
cache = true;
}
filter = new TermRangeFilter(fieldName, BytesRefs.toBytesRef(from), BytesRefs.toBytesRef(to), includeLower, includeUpper);
}

Expand Down
24 changes: 18 additions & 6 deletions src/test/java/org/elasticsearch/count/query/SimpleQueryTests.java
Expand Up @@ -1031,16 +1031,16 @@ public void testNumericRangeFilter_2826() throws Exception {
client().admin().indices().prepareRefresh().execute().actionGet();
CountResponse response = client().prepareCount("test").setQuery(
filteredQuery(matchAllQuery(), FilterBuilders.boolFilter()
.should(FilterBuilders.rangeFilter("num_long").from(1).to(2))
.should(FilterBuilders.rangeFilter("num_long").from(3).to(4)))
.should(rangeFilter("num_long", 1, 2))
.should(rangeFilter("num_long", 3, 4)))
).execute().actionGet();
assertHitCount(response, 4l);

// This made 2826 fail! (only with bit based filters)
response = client().prepareCount("test").setQuery(
filteredQuery(matchAllQuery(), FilterBuilders.boolFilter()
.should(FilterBuilders.numericRangeFilter("num_long").from(1).to(2))
.should(FilterBuilders.numericRangeFilter("num_long").from(3).to(4)))
.should(rangeFilter("num_long", 1, 2))
.should(rangeFilter("num_long", 3, 4)))
).execute().actionGet();

assertHitCount(response, 4l);
Expand All @@ -1049,8 +1049,8 @@ public void testNumericRangeFilter_2826() throws Exception {
response = client().prepareCount("test").setQuery(
filteredQuery(matchAllQuery(), FilterBuilders.boolFilter()
.must(FilterBuilders.termFilter("field1", "test1"))
.should(FilterBuilders.rangeFilter("num_long").from(1).to(2))
.should(FilterBuilders.rangeFilter("num_long").from(3).to(4)))
.should(rangeFilter("num_long", 1, 2))
.should(rangeFilter("num_long", 3, 4)))
).execute().actionGet();

assertHitCount(response, 2l);
Expand Down Expand Up @@ -1108,4 +1108,16 @@ public void testSimpleSpan() throws ElasticSearchException, IOException {
assertNoFailures(response);
assertHitCount(response, 3l);
}

public static FilterBuilder rangeFilter(String field, Object from, Object to) {
if (randomBoolean()) {
if (randomBoolean()) {
return FilterBuilders.rangeFilter(field).from(from).to(to);
} else {
return FilterBuilders.rangeFilter(field).from(from).to(to).setExecution("fielddata");
}
} else {
return FilterBuilders.numericRangeFilter(field).from(from).to(to);
}
}
}
Expand Up @@ -922,6 +922,21 @@ public void testNumericRangeFilteredQueryBuilder() throws IOException {
assertThat(rangeFilter.isIncludeUpper(), equalTo(false));
}

@Test
public void testRangeFilteredQueryBuilder_executionFieldData() throws IOException {
IndexQueryParserService queryParser = queryParser();
Query parsedQuery = queryParser.parse(filteredQuery(termQuery("name.first", "shay"), rangeFilter("age").from(23).to(54).includeLower(true).includeUpper(false).setExecution("fielddata"))).query();
assertThat(parsedQuery, instanceOf(XFilteredQuery.class));
Filter filter = ((XFilteredQuery) parsedQuery).getFilter();
assertThat(filter, instanceOf(NumericRangeFieldDataFilter.class));
NumericRangeFieldDataFilter<Number> rangeFilter = (NumericRangeFieldDataFilter<Number>) filter;
assertThat(rangeFilter.getField(), equalTo("age"));
assertThat(rangeFilter.getLowerVal().intValue(), equalTo(23));
assertThat(rangeFilter.getUpperVal().intValue(), equalTo(54));
assertThat(rangeFilter.isIncludeLower(), equalTo(true));
assertThat(rangeFilter.isIncludeUpper(), equalTo(false));
}

@Test
public void testNumericRangeFilteredQuery() throws IOException {
IndexQueryParserService queryParser = queryParser();
Expand Down
Expand Up @@ -51,6 +51,7 @@
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.count.query.SimpleQueryTests.rangeFilter;
import static org.elasticsearch.index.query.FilterBuilders.*;
import static org.elasticsearch.index.query.QueryBuilders.*;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*;
Expand Down Expand Up @@ -1480,16 +1481,16 @@ public void testNumericRangeFilter_2826() throws Exception {
client().admin().indices().prepareRefresh().execute().actionGet();
SearchResponse response = client().prepareSearch("test").setFilter(
FilterBuilders.boolFilter()
.should(FilterBuilders.rangeFilter("num_long").from(1).to(2))
.should(FilterBuilders.rangeFilter("num_long").from(3).to(4))
.should(rangeFilter("num_long", 1, 2))
.should(rangeFilter("num_long", 3, 4))
).execute().actionGet();
assertThat(response.getHits().totalHits(), equalTo(4l));

// This made 2826 fail! (only with bit based filters)
response = client().prepareSearch("test").setFilter(
FilterBuilders.boolFilter()
.should(FilterBuilders.numericRangeFilter("num_long").from(1).to(2))
.should(FilterBuilders.numericRangeFilter("num_long").from(3).to(4))
.should(rangeFilter("num_long", 1, 2))
.should(rangeFilter("num_long", 3, 4))
).execute().actionGet();

assertThat(response.getHits().totalHits(), equalTo(4l));
Expand All @@ -1498,8 +1499,8 @@ public void testNumericRangeFilter_2826() throws Exception {
response = client().prepareSearch("test").setFilter(
FilterBuilders.boolFilter()
.must(FilterBuilders.termFilter("field1", "test1"))
.should(FilterBuilders.rangeFilter("num_long").from(1).to(2))
.should(FilterBuilders.rangeFilter("num_long").from(3).to(4))
.should(rangeFilter("num_long", 1, 2))
.should(rangeFilter("num_long", 3, 4))
).execute().actionGet();

assertThat(response.getHits().totalHits(), equalTo(2l));
Expand Down