Skip to content

Commit

Permalink
Query DSL: Cache range filter on date field by default
Browse files Browse the repository at this point in the history 
A range filter on a date field with a numeric `from`/`to` value is **not** cached by default:

    DELETE /test

    PUT /test/t/1
    {
      "date": "2014-01-01"
    }

    GET /_validate/query?explain
    {
      "query": {
        "filtered": {
          "filter": {
            "range": {
              "date": {
                "from": 0
              }
            }
          }
        }
      }
    }

Returns:

    "explanation": "ConstantScore(no_cache(date:[0 TO *]))"

This patch fixes as well not caching `from`/`to` when using `now` value not rounded.
Previously, a query like:

    GET /_validate/query?explain
    {
      "query": {
        "filtered": {
          "filter": {
            "range": {
              "date": {
                "from": "now"
                "to": "now/d+1"
              }
            }
          }
        }
      }
    }

was cached.

Also, this patch does not cache anymore `now` even if the user asked for caching it.
As it won't be cached at all by definition.

Added as well tests for all possible combinations.

Closes #7114.
  • Loading branch information
@dadoonet @areek
dadoonet authored and areek committed Sep 8, 2014
1 parent a8b31bc commit d56ccef
Show file tree
Hide file tree
Showing 7 changed files with 204 additions and 14 deletions.
27 changes: 19 additions & 8 deletions src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java
View file
Expand Up @@ -336,16 +336,16 @@ 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 rangeFilter(lowerTerm, upperTerm, includeLower, includeUpper, null, context, false);
return rangeFilter(lowerTerm, upperTerm, includeLower, includeUpper, null, context, null);
}

public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable DateTimeZone timeZone, @Nullable QueryParseContext context, boolean explicitCaching) {
public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable DateTimeZone timeZone, @Nullable QueryParseContext context, @Nullable Boolean explicitCaching) {
return rangeFilter(null, lowerTerm, upperTerm, includeLower, includeUpper, timeZone, context, explicitCaching);
}

@Override
public Filter rangeFilter(QueryParseContext parseContext, Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
return rangeFilter(parseContext, lowerTerm, upperTerm, includeLower, includeUpper, null, context, false);
return rangeFilter(parseContext, lowerTerm, upperTerm, includeLower, includeUpper, null, context, null);
}

/*
Expand All @@ -354,16 +354,17 @@ public Filter rangeFilter(QueryParseContext parseContext, Object lowerTerm, Obje
* - the object to parse is a String (does not apply to ms since epoch which are UTC based time values)
* - the String to parse does not have already a timezone defined (ie. `2014-01-01T00:00:00+03:00`)
*/
public Filter rangeFilter(QueryParseContext parseContext, Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable DateTimeZone timeZone, @Nullable QueryParseContext context, boolean explicitCaching) {
boolean cache = explicitCaching;
public Filter rangeFilter(QueryParseContext parseContext, Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable DateTimeZone timeZone, @Nullable QueryParseContext context, @Nullable Boolean explicitCaching) {
boolean cache;
boolean cacheable = true;
Long lowerVal = null;
Long upperVal = null;
if (lowerTerm != null) {
if (lowerTerm instanceof Number) {
lowerVal = ((Number) lowerTerm).longValue();
} else {
String value = convertToString(lowerTerm);
cache = explicitCaching || !hasNowExpressionWithNoRounding(value);
cacheable = !hasDateExpressionWithNoRounding(value);
lowerVal = parseToMilliseconds(value, context, false, timeZone);
}
}
Expand All @@ -372,11 +373,21 @@ public Filter rangeFilter(QueryParseContext parseContext, Object lowerTerm, Obje
upperVal = ((Number) upperTerm).longValue();
} else {
String value = convertToString(upperTerm);
cache = explicitCaching || !hasNowExpressionWithNoRounding(value);
cacheable = cacheable && !hasDateExpressionWithNoRounding(value);
upperVal = parseToMilliseconds(value, context, includeUpper, timeZone);
}
}

if (explicitCaching != null) {
if (explicitCaching) {
cache = cacheable;
} else {
cache = false;
}
} else {
cache = cacheable;
}

Filter filter;
if (parseContext != null) {
filter = NumericRangeFieldDataFilter.newLongRange(
Expand All @@ -397,7 +408,7 @@ public Filter rangeFilter(QueryParseContext parseContext, Object lowerTerm, Obje
}
}

private boolean hasNowExpressionWithNoRounding(String value) {
private boolean hasDateExpressionWithNoRounding(String value) {
int index = value.indexOf("now");
if (index != -1) {
if (value.length() == 3) {
Expand Down
8 changes: 5 additions & 3 deletions src/main/java/org/elasticsearch/index/query/RangeFilterParser.java
View file
Expand Up @@ -125,10 +125,10 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
}

Filter filter = null;
Boolean explicitlyCached = cache;
MapperService.SmartNameFieldMappers smartNameFieldMappers = parseContext.smartFieldMappers(fieldName);
if (smartNameFieldMappers != null) {
if (smartNameFieldMappers.hasMapper()) {
boolean explicitlyCached = cache != null && cache;
if (execution.equals("index")) {
if (cache == null) {
cache = true;
Expand Down Expand Up @@ -177,8 +177,10 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
filter = new TermRangeFilter(fieldName, BytesRefs.toBytesRef(from), BytesRefs.toBytesRef(to), includeLower, includeUpper);
}

if (cache) {
filter = parseContext.cacheFilter(filter, cacheKey);
if (explicitlyCached == null || explicitlyCached) {
if (cache) {
filter = parseContext.cacheFilter(filter, cacheKey);
}
}

filter = wrapSmartNameFilter(filter, smartNameFieldMappers, parseContext);
Expand Down
126 changes: 126 additions & 0 deletions src/test/java/org/elasticsearch/index/query/IndexQueryParserFilterCachingTests.java
View file
Expand Up @@ -76,6 +76,118 @@ private IndexQueryParserService queryParser() throws IOException {
return this.queryParser;
}

/**
* Runner to test our cache cases when using date range filter
* @param lte could be null
* @param gte could be null
* @param forcedCache true if we want to force the cache, false if we want to force no cache, null either
* @param expectedCache true if we expect a cached filter
*/
private void testDateRangeFilterCache(IndexQueryParserService queryParser, Object gte, Object lte, Boolean forcedCache, boolean expectedCache) {
RangeFilterBuilder filterBuilder = FilterBuilders.rangeFilter("born")
.gte(gte)
.lte(lte);
if (forcedCache != null) {
filterBuilder.cache(forcedCache);
}

Query parsedQuery = queryParser.parse(QueryBuilders.constantScoreQuery(filterBuilder)).query();
assertThat(parsedQuery, instanceOf(ConstantScoreQuery.class));


if (expectedCache) {
if (((ConstantScoreQuery)parsedQuery).getFilter() instanceof CachedFilter) {
logger.info("gte [{}], lte [{}], _cache [{}] is cached", gte, lte, forcedCache);
} else {
logger.warn("gte [{}], lte [{}], _cache [{}] should be cached", gte, lte, forcedCache);
}
} else {
if (((ConstantScoreQuery)parsedQuery).getFilter() instanceof NoCacheFilter) {
logger.info("gte [{}], lte [{}], _cache [{}] is not cached", gte, lte, forcedCache);
} else {
logger.warn("gte [{}], lte [{}], _cache [{}] should not be cached", gte, lte, forcedCache);
}
}

if (expectedCache) {
assertThat(((ConstantScoreQuery)parsedQuery).getFilter(), instanceOf(CachedFilter.class));
} else {
assertThat(((ConstantScoreQuery)parsedQuery).getFilter(), instanceOf(NoCacheFilter.class));
}
}

/**
* We test all possible combinations for range date filter cache
*/
@Test
public void testDateRangeFilterCache() throws IOException {
IndexQueryParserService queryParser = queryParser();

testDateRangeFilterCache(queryParser, null, null, null, true);
testDateRangeFilterCache(queryParser, null, null, true, true);
testDateRangeFilterCache(queryParser, null, null, false, false);
testDateRangeFilterCache(queryParser, "now", null, null, false);
testDateRangeFilterCache(queryParser, null, "now", null, false);
testDateRangeFilterCache(queryParser, "now", "now", null, false);
testDateRangeFilterCache(queryParser, "now/d", null, null, true);
testDateRangeFilterCache(queryParser, null, "now/d", null, true);
testDateRangeFilterCache(queryParser, "now/d", "now/d", null, true);
testDateRangeFilterCache(queryParser, "2012-01-01", null, null, true);
testDateRangeFilterCache(queryParser, null, "2012-01-01", null, true);
testDateRangeFilterCache(queryParser, "2012-01-01", "2012-01-01", null, true);
testDateRangeFilterCache(queryParser, "now", "2012-01-01", null, false);
testDateRangeFilterCache(queryParser, "2012-01-01", "now", null, false);
testDateRangeFilterCache(queryParser, "2012-01-01", "now/d", null, true);
testDateRangeFilterCache(queryParser, "now/d", "2012-01-01", null, true);
testDateRangeFilterCache(queryParser, null, 1577836800, null, true);
testDateRangeFilterCache(queryParser, 1325376000, null, null, true);
testDateRangeFilterCache(queryParser, 1325376000, 1577836800, null, true);
testDateRangeFilterCache(queryParser, "now", 1577836800, null, false);
testDateRangeFilterCache(queryParser, 1325376000, "now", null, false);
testDateRangeFilterCache(queryParser, 1325376000, "now/d", null, true);
testDateRangeFilterCache(queryParser, "now/d", 1577836800, null, true);
testDateRangeFilterCache(queryParser, "now", null, true, false);
testDateRangeFilterCache(queryParser, null, "now", true, false);
testDateRangeFilterCache(queryParser, "now", "now", true, false);
testDateRangeFilterCache(queryParser, "now/d", null, true, true);
testDateRangeFilterCache(queryParser, null, "now/d", true, true);
testDateRangeFilterCache(queryParser, "now/d", "now/d", true, true);
testDateRangeFilterCache(queryParser, "2012-01-01", null, true, true);
testDateRangeFilterCache(queryParser, null, "2012-01-01", true, true);
testDateRangeFilterCache(queryParser, "2012-01-01", "2012-01-01", true, true);
testDateRangeFilterCache(queryParser, "now", "2012-01-01", true, false);
testDateRangeFilterCache(queryParser, "2012-01-01", "now", true, false);
testDateRangeFilterCache(queryParser, "2012-01-01", "now/d", true, true);
testDateRangeFilterCache(queryParser, "now/d", "2012-01-01", true, true);
testDateRangeFilterCache(queryParser, null, 1577836800, true, true);
testDateRangeFilterCache(queryParser, 1325376000, null, true, true);
testDateRangeFilterCache(queryParser, 1325376000, 1577836800, true, true);
testDateRangeFilterCache(queryParser, "now", 1577836800, true, false);
testDateRangeFilterCache(queryParser, 1325376000, "now", true, false);
testDateRangeFilterCache(queryParser, 1325376000, "now/d", true, true);
testDateRangeFilterCache(queryParser, "now/d", 1577836800, true, true);
testDateRangeFilterCache(queryParser, "now", null, false, false);
testDateRangeFilterCache(queryParser, null, "now", false, false);
testDateRangeFilterCache(queryParser, "now", "now", false, false);
testDateRangeFilterCache(queryParser, "now/d", null, false, false);
testDateRangeFilterCache(queryParser, null, "now/d", false, false);
testDateRangeFilterCache(queryParser, "now/d", "now/d", false, false);
testDateRangeFilterCache(queryParser, "2012-01-01", null, false, false);
testDateRangeFilterCache(queryParser, null, "2012-01-01", false, false);
testDateRangeFilterCache(queryParser, "2012-01-01", "2012-01-01", false, false);
testDateRangeFilterCache(queryParser, "now", "2012-01-01", false, false);
testDateRangeFilterCache(queryParser, "2012-01-01", "now", false, false);
testDateRangeFilterCache(queryParser, "2012-01-01", "now/d", false, false);
testDateRangeFilterCache(queryParser, "now/d", "2012-01-01", false, false);
testDateRangeFilterCache(queryParser, null, 1577836800, false, false);
testDateRangeFilterCache(queryParser, 1325376000, null, false, false);
testDateRangeFilterCache(queryParser, 1325376000, 1577836800, false, false);
testDateRangeFilterCache(queryParser, "now", 1577836800, false, false);
testDateRangeFilterCache(queryParser, 1325376000, "now", false, false);
testDateRangeFilterCache(queryParser, 1325376000, "now/d", false, false);
testDateRangeFilterCache(queryParser, "now/d", 1577836800, false, false);
}

@Test
public void testNoFilterParsing() throws IOException {
IndexQueryParserService queryParser = queryParser();
Expand All @@ -86,6 +198,20 @@ public void testNoFilterParsing() throws IOException {
assertThat(((XBooleanFilter) ((ConstantScoreQuery) parsedQuery).getFilter()).clauses().get(1).getFilter(), instanceOf(NoCacheFilter.class));
assertThat(((XBooleanFilter) ((ConstantScoreQuery) parsedQuery).getFilter()).clauses().size(), is(2));

query = copyToStringFromClasspath("/org/elasticsearch/index/query/date_range_in_boolean_with_long_value.json");
parsedQuery = queryParser.parse(query).query();
assertThat(parsedQuery, instanceOf(ConstantScoreQuery.class));
assertThat(((ConstantScoreQuery) parsedQuery).getFilter(), instanceOf(XBooleanFilter.class));
assertThat(((XBooleanFilter) ((ConstantScoreQuery) parsedQuery).getFilter()).clauses().get(1).getFilter(), instanceOf(CachedFilter.class));
assertThat(((XBooleanFilter) ((ConstantScoreQuery) parsedQuery).getFilter()).clauses().size(), is(2));

query = copyToStringFromClasspath("/org/elasticsearch/index/query/date_range_in_boolean_with_long_value_not_cached.json");
parsedQuery = queryParser.parse(query).query();
assertThat(parsedQuery, instanceOf(ConstantScoreQuery.class));
assertThat(((ConstantScoreQuery) parsedQuery).getFilter(), instanceOf(XBooleanFilter.class));
assertThat(((XBooleanFilter) ((ConstantScoreQuery) parsedQuery).getFilter()).clauses().get(1).getFilter(), instanceOf(NoCacheFilter.class));
assertThat(((XBooleanFilter) ((ConstantScoreQuery) parsedQuery).getFilter()).clauses().size(), is(2));

query = copyToStringFromClasspath("/org/elasticsearch/index/query/date_range_in_boolean_cached_now.json");
parsedQuery = queryParser.parse(query).query();
assertThat(parsedQuery, instanceOf(ConstantScoreQuery.class));
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/elasticsearch/index/query/date_range_in_boolean_cached.json
View file
Expand Up @@ -23,4 +23,4 @@
}
}
}
}
}
25 changes: 25 additions & 0 deletions src/test/java/org/elasticsearch/index/query/date_range_in_boolean_with_long_value.json
View file
@@ -0,0 +1,25 @@
{
"constant_score": {
"filter": {
"bool": {
"must": [
{
"term": {
"foo": {
"value": "bar"
}
}
},
{
"range" : {
"born" : {
"gte": 1325376000,
"lte": 1577836800
}
}
}
]
}
}
}
}
26 changes: 26 additions & 0 deletions .../java/org/elasticsearch/index/query/date_range_in_boolean_with_long_value_not_cached.json
View file
@@ -0,0 +1,26 @@
{
"constant_score": {
"filter": {
"bool": {
"must": [
{
"term": {
"foo": {
"value": "bar"
}
}
},
{
"range" : {
"_cache" : false,
"born" : {
"gte": 1325376000,
"lte": 1577836800
}
}
}
]
}
}
}
}
4 changes: 2 additions & 2 deletions src/test/java/org/elasticsearch/search/query/SimpleQueryTests.java
View file
Expand Up @@ -2340,9 +2340,9 @@ public void testRangeFilterNoCacheWithNow() throws Exception {
.get();
assertHitCount(searchResponse, 1l);

// The range filter is now explicitly cached, so it now it is in the filter cache.
// The range filter is now explicitly cached but we don't want to cache now even if the user asked for it
statsResponse = client().admin().indices().prepareStats("test").clear().setFilterCache(true).get();
assertThat(statsResponse.getIndex("test").getTotal().getFilterCache().getMemorySizeInBytes(), cluster().hasFilterCache() ? greaterThan(filtercacheSize) : is(filtercacheSize));
assertThat(statsResponse.getIndex("test").getTotal().getFilterCache().getMemorySizeInBytes(), is(filtercacheSize));
}

@Test
Expand Down

0 comments on commit d56ccef

Please sign in to comment.