Skip to content

Commit

Permalink
Never cache a range filter that uses the now date expression.
Browse files Browse the repository at this point in the history 
Closes #4846
  • Loading branch information
@martijnvg
martijnvg committed Jan 22, 2014
1 parent 06a54d5 commit 45a68e2
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 8 deletions.
62 changes: 54 additions & 8 deletions src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java
View file
Expand Up @@ -35,6 +35,7 @@
import org.elasticsearch.common.joda.DateMathParser;
import org.elasticsearch.common.joda.FormatDateTimeFormatter;
import org.elasticsearch.common.joda.Joda;
import org.elasticsearch.common.lucene.search.NoCacheFilter;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.common.xcontent.XContentBuilder;
Expand Down Expand Up @@ -324,6 +325,11 @@ public long parseToMilliseconds(Object value, @Nullable QueryParseContext contex
return includeUpper && roundCeil ? dateMathParser.parseRoundCeil(convertToString(value), now) : dateMathParser.parse(convertToString(value), now);
}

public long parseToMilliseconds(String value, @Nullable QueryParseContext context, boolean includeUpper) {
long now = context == null ? System.currentTimeMillis() : context.nowInMillis();
return includeUpper && roundCeil ? dateMathParser.parseRoundCeil(value, now) : dateMathParser.parse(value, now);
}

@Override
public Filter termFilter(Object value, @Nullable QueryParseContext context) {
final long lValue = parseToMilliseconds(value, context);
Expand All @@ -341,18 +347,58 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower

@Override
public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeFilter.newLongRange(names.indexName(), precisionStep,
lowerTerm == null ? null : parseToMilliseconds(lowerTerm, context),
upperTerm == null ? null : parseToMilliseconds(upperTerm, context, includeUpper),
includeLower, includeUpper);
boolean nowIsUsed = false;
Long lowerVal = null;
Long upperVal = null;
if (lowerTerm != null) {
String value = convertToString(lowerTerm);
nowIsUsed = value.contains("now");
lowerVal = parseToMilliseconds(value, context, false);
}
if (upperTerm != null) {
String value = convertToString(upperTerm);
nowIsUsed = value.contains("now");
upperVal = parseToMilliseconds(value, context, includeUpper);
}

Filter filter = NumericRangeFilter.newLongRange(
names.indexName(), precisionStep, lowerVal, upperVal, includeLower, includeUpper
);
if (nowIsUsed) {
// We don't cache range filter if `now` date expression is used and also when a compound filter wraps
// a range filter with a `now` date expressions.
return NoCacheFilter.wrap(filter);
} else {
return filter;
}
}

@Override
public Filter rangeFilter(IndexFieldDataService fieldData, Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return NumericRangeFieldDataFilter.newLongRange((IndexNumericFieldData<?>) fieldData.getForField(this),
lowerTerm == null ? null : parseToMilliseconds(lowerTerm, context),
upperTerm == null ? null : parseToMilliseconds(upperTerm, context, includeUpper),
includeLower, includeUpper);
boolean nowIsUsed = false;
Long lowerVal = null;
Long upperVal = null;
if (lowerTerm != null) {
String value = convertToString(lowerTerm);
nowIsUsed = value.contains("now");
lowerVal = parseToMilliseconds(value, context, false);
}
if (upperTerm != null) {
String value = convertToString(upperTerm);
nowIsUsed = value.contains("now");
upperVal = parseToMilliseconds(value, context, includeUpper);
}

Filter filter = NumericRangeFieldDataFilter.newLongRange(
(IndexNumericFieldData<?>) fieldData.getForField(this), lowerVal,upperVal, includeLower, includeUpper
);
if (nowIsUsed) {
// We don't cache range filter if `now` date expression is used and also when a compound filter wraps
// a range filter with a `now` date expressions.
return NoCacheFilter.wrap(filter);
} else {
return filter;
}
}

@Override
Expand Down
50 changes: 50 additions & 0 deletions src/test/java/org/elasticsearch/search/query/SimpleQueryTests.java
View file
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse;
Expand Down Expand Up @@ -2033,6 +2034,55 @@ public void testSimpleQueryStringFlags() {
assertFirstHit(searchResponse, hasId("4"));
}

@Test
public void testRangeFilterNoCacheWithNow() throws Exception {
assertAcked(client().admin().indices().prepareCreate("test")
.setSettings(SETTING_NUMBER_OF_SHARDS, 1, SETTING_NUMBER_OF_REPLICAS, 0)
.addMapping("type1", "date", "type=date,format=YYYY-mm-dd"));
ensureGreen();

client().prepareIndex("test", "type1", "1").setSource("date", "2014-01-01", "field", "value")
.setRefresh(true)
.get();

SearchResponse searchResponse = client().prepareSearch("test")
.setQuery(QueryBuilders.filteredQuery(matchAllQuery(), FilterBuilders.rangeFilter("date").from("2013-01-01").to("now").cache(true)))
.get();
assertHitCount(searchResponse, 1l);

// filter cache should not contain any thing, b/c `now` is used in `to`.
IndicesStatsResponse statsResponse = client().admin().indices().prepareStats("test").clear().setFilterCache(true).get();
assertThat(statsResponse.getIndex("test").getTotal().getFilterCache().getMemorySizeInBytes(), equalTo(0l));

searchResponse = client().prepareSearch("test")
.setQuery(QueryBuilders.filteredQuery(
matchAllQuery(),
FilterBuilders.boolFilter().cache(true)
.must(FilterBuilders.matchAllFilter())
.must(FilterBuilders.rangeFilter("date").from("2013-01-01").to("now").cache(true))
))
.get();
assertHitCount(searchResponse, 1l);

// filter cache should not contain any thing, b/c `now` is used in `to`.
statsResponse = client().admin().indices().prepareStats("test").clear().setFilterCache(true).get();
assertThat(statsResponse.getIndex("test").getTotal().getFilterCache().getMemorySizeInBytes(), equalTo(0l));

searchResponse = client().prepareSearch("test")
.setQuery(QueryBuilders.filteredQuery(
matchAllQuery(),
FilterBuilders.boolFilter().cache(true)
.must(FilterBuilders.termFilter("field", "value").cache(true))
.must(FilterBuilders.rangeFilter("date").from("2013-01-01").to("now").cache(true))
))
.get();
assertHitCount(searchResponse, 1l);

// filter cache only has a cache entry for the term filter
statsResponse = client().admin().indices().prepareStats("test").clear().setFilterCache(true).get();
assertThat(statsResponse.getIndex("test").getTotal().getFilterCache().getMemorySizeInBytes(), greaterThan(0l));
}

@Test
public void testSearchEmptyDoc() {
prepareCreate("test").setSettings("{\"index.analysis.analyzer.default.type\":\"keyword\"}").get();
Expand Down

0 comments on commit 45a68e2

Please sign in to comment.