The forceful no cache behaviour for range filter with now date match …

…expression should only be active if no rounding has been specified for `now` in the date range range expression (for example: `now/d`).

Also the automatic now detection in range filters is overrideable by the `_cache` option.

 Closes elastic#4947
 Relates to elastic#4846

docs/reference/query-dsl/filters/numeric-range-filter.asciidoc

@@ -56,3 +56,6 @@ If caching the *result* of the filter is desired (for example, using the
 same "teen" filter with ages between 10 and 20), then it is advisable to
 simply use the <<query-dsl-range-filter,range>>
 filter.
+
+If the `now` date math expression is used without rounding then a range numeric filter will never be cached even
+if `_cache` is set to `true`. Also any filter that wraps this filter will never be cached.

docs/reference/query-dsl/filters/range-filter.asciidoc

@@ -54,3 +54,6 @@ already faceting or sorting by.
 
 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.
+
+If the `now` date math expression is used without rounding then a range filter will never be cached even if `_cache` is
+set to `true`. Also any filter that wraps this filter will never be cached.

src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java

@@ -334,24 +334,28 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
 
     @Override
     public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
-        boolean nowIsUsed = false;
+        return rangeFilter(lowerTerm, upperTerm, includeLower, includeUpper, context, false);
+    }
+
+    public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context, boolean explicitCaching) {
+        boolean cache = explicitCaching;
         Long lowerVal = null;
         Long upperVal = null;
         if (lowerTerm != null) {
             String value = convertToString(lowerTerm);
-            nowIsUsed = value.contains("now");
+            cache = explicitCaching || !hasNowExpressionWithNoRounding(value);
             lowerVal = parseToMilliseconds(value, context, false);
         }
         if (upperTerm != null) {
             String value = convertToString(upperTerm);
-            nowIsUsed = value.contains("now");
+            cache = explicitCaching || !hasNowExpressionWithNoRounding(value);
             upperVal = parseToMilliseconds(value, context, includeUpper);
         }
 
         Filter filter =  NumericRangeFilter.newLongRange(
             names.indexName(), precisionStep, lowerVal, upperVal, includeLower, includeUpper
         );
-        if (nowIsUsed) {
+        if (!cache) {
             // 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);
@@ -362,24 +366,28 @@ public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLow
 
     @Override
     public Filter rangeFilter(IndexFieldDataService fieldData, Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
-        boolean nowIsUsed = false;
+        return rangeFilter(fieldData, lowerTerm, upperTerm, includeLower, includeUpper, context, false);
+    }
+
+    public Filter rangeFilter(IndexFieldDataService fieldData, Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context, boolean explicitCaching) {
+        boolean cache = explicitCaching;
         Long lowerVal = null;
         Long upperVal = null;
         if (lowerTerm != null) {
             String value = convertToString(lowerTerm);
-            nowIsUsed = value.contains("now");
+            cache = explicitCaching || !hasNowExpressionWithNoRounding(value);
             lowerVal = parseToMilliseconds(value, context, false);
         }
         if (upperTerm != null) {
             String value = convertToString(upperTerm);
-            nowIsUsed = value.contains("now");
+            cache = explicitCaching || !hasNowExpressionWithNoRounding(value);
             upperVal = parseToMilliseconds(value, context, includeUpper);
         }
 
         Filter filter =  NumericRangeFieldDataFilter.newLongRange(
             (IndexNumericFieldData<?>) fieldData.getForField(this), lowerVal,upperVal, includeLower, includeUpper
         );
-        if (nowIsUsed) {
+        if (!cache) {
             // 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);
@@ -388,6 +396,33 @@ public Filter rangeFilter(IndexFieldDataService fieldData, Object lowerTerm, Obj
         }
     }
 
+    private boolean hasNowExpressionWithNoRounding(String value) {
+        int index = value.indexOf("now");
+        if (index != -1) {
+            if (value.length() == 3) {
+                return true;
+            } else {
+                int indexOfPotentialRounding = index + 3;
+                if (indexOfPotentialRounding >= value.length()) {
+                    return true;
+                } else {
+                    char potentialRoundingChar;
+                    do {
+                        potentialRoundingChar = value.charAt(indexOfPotentialRounding++);
+                        if (potentialRoundingChar == '/') {
+                            return false; // We found the rounding char, so we shouldn't forcefully disable caching
+                        } else if (potentialRoundingChar == ' ') {
+                            return true; // Next token in the date math expression and no rounding found, so we should not cache.
+                        }
+                    } while (indexOfPotentialRounding < value.length());
+                    return true; // Couldn't find rounding char, so we should not cache
+                }
+            }
+        } else {
+            return false;
+        }
+    }
+
     @Override
     public Filter nullValueFilter() {
         if (nullValue == null) {

src/main/java/org/elasticsearch/index/query/RangeFilterParser.java

@@ -27,6 +27,7 @@
 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.DateFieldMapper;
 import org.elasticsearch.index.mapper.core.NumberFieldMapper;
 
 import java.io.IOException;
@@ -122,11 +123,17 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
         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;
                     }
-                    filter = smartNameFieldMappers.mapper().rangeFilter(from, to, includeLower, includeUpper, parseContext);
+                    FieldMapper mapper = smartNameFieldMappers.mapper();
+                    if (mapper instanceof DateFieldMapper) {
+                        filter = ((DateFieldMapper) mapper).rangeFilter(from, to, includeLower, includeUpper, parseContext, explicitlyCached);
+                    } else  {
+                        filter = mapper.rangeFilter(from, to, includeLower, includeUpper, parseContext);
+                    }
                 } else if ("fielddata".equals(execution)) {
                     if (cache == null) {
                         cache = false;
@@ -135,7 +142,11 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
                     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);
+                    if (mapper instanceof DateFieldMapper) {
+                        filter = ((DateFieldMapper) mapper).rangeFilter(parseContext.fieldData(), from, to, includeLower, includeUpper, parseContext, explicitlyCached);
+                    } else {
+                        filter = ((NumberFieldMapper) mapper).rangeFilter(parseContext.fieldData(), from, to, includeLower, includeUpper, parseContext);
+                    }
                 } else {
                     throw new QueryParsingException(parseContext.index(), "[range] filter doesn't support [" + execution + "] execution");
                 }

src/test/java/org/elasticsearch/index/query/IndexQueryParserFilterCachingTests.java

@@ -145,9 +145,26 @@ 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_cached_complex_now.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.json");
         parsedQuery = queryParser.parse(query).query();
         assertThat(parsedQuery, instanceOf(ConstantScoreQuery.class));
+        assertThat(((ConstantScoreQuery) parsedQuery).getFilter(), instanceOf(CachedFilter.class));
+
+        query = copyToStringFromClasspath("/org/elasticsearch/index/query/date_range_in_boolean_cached_now_with_rounding.json");
+        parsedQuery = queryParser.parse(query).query();
+        assertThat(parsedQuery, instanceOf(ConstantScoreQuery.class));
+        assertThat(((ConstantScoreQuery) parsedQuery).getFilter(), instanceOf(CachedFilter.class));
+
+        query = copyToStringFromClasspath("/org/elasticsearch/index/query/date_range_in_boolean_cached_complex_now_with_rounding.json");
+        parsedQuery = queryParser.parse(query).query();
+        assertThat(parsedQuery, instanceOf(ConstantScoreQuery.class));
         assertThat(((ConstantScoreQuery)parsedQuery).getFilter(), instanceOf(CachedFilter.class));
     }
 

src/test/java/org/elasticsearch/index/query/date_range_in_boolean_cached_complex_now.json

@@ -0,0 +1,26 @@
+{
+    "constant_score": {
+        "filter": {
+            "bool": {
+                "_cache" : true,
+                "must": [
+                    {
+                        "term": {
+                            "foo": {
+                                "value": "bar"
+                            }
+                        }
+                    },
+                    {
+                        "range" : {
+                            "born" : {
+                                "gte": "2012-01-01",
+                                "lte": "now+1m+1s"
+                            }
+                        }
+                    }
+                ]
+            }
+        }
+    }
+}

src/test/java/org/elasticsearch/index/query/date_range_in_boolean_cached_complex_now_with_rounding.json

@@ -0,0 +1,26 @@
+{
+    "constant_score": {
+        "filter": {
+            "bool": {
+                "_cache" : true,
+                "must": [
+                    {
+                        "term": {
+                            "foo": {
+                                "value": "bar"
+                            }
+                        }
+                    },
+                    {
+                        "range" : {
+                            "born" : {
+                                "gte": "2012-01-01",
+                                "lte": "now+1m+1s/m"
+                            }
+                        }
+                    }
+                ]
+            }
+        }
+    }
+}

src/test/java/org/elasticsearch/index/query/date_range_in_boolean_cached_now_with_rounding.json

@@ -0,0 +1,26 @@
+{
+    "constant_score": {
+        "filter": {
+            "bool": {
+                "_cache" : true,
+                "must": [
+                    {
+                        "term": {
+                            "foo": {
+                                "value": "bar"
+                            }
+                        }
+                    },
+                    {
+                        "range" : {
+                            "born" : {
+                                "gte": "2012-01-01",
+                                "lte": "now/d"
+                            }
+                        }
+                    }
+                ]
+            }
+        }
+    }
+}

src/test/java/org/elasticsearch/search/query/SimpleQueryTests.java

@@ -2030,7 +2030,7 @@ public void testRangeFilterNoCacheWithNow() throws Exception {
                 .get();
 
         SearchResponse searchResponse = client().prepareSearch("test")
-                .setQuery(QueryBuilders.filteredQuery(matchAllQuery(), FilterBuilders.rangeFilter("date").from("2013-01-01").to("now").cache(true)))
+                .setQuery(QueryBuilders.filteredQuery(matchAllQuery(), FilterBuilders.rangeFilter("date").from("2013-01-01").to("now")))
                 .get();
         assertHitCount(searchResponse, 1l);
 
@@ -2043,7 +2043,7 @@ public void testRangeFilterNoCacheWithNow() throws Exception {
                         matchAllQuery(),
                         FilterBuilders.boolFilter().cache(true)
                                 .must(FilterBuilders.matchAllFilter())
-                                .must(FilterBuilders.rangeFilter("date").from("2013-01-01").to("now").cache(true))
+                                .must(FilterBuilders.rangeFilter("date").from("2013-01-01").to("now"))
                 ))
                 .get();
         assertHitCount(searchResponse, 1l);
@@ -2052,19 +2052,49 @@ public void testRangeFilterNoCacheWithNow() throws Exception {
         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/d").cache(true))
+                ))
+                .get();
+        assertHitCount(searchResponse, 1l);
+        // Now with rounding is used, so we must have something in filter cache
+        statsResponse = client().admin().indices().prepareStats("test").clear().setFilterCache(true).get();
+        long filtercacheSize = statsResponse.getIndex("test").getTotal().getFilterCache().getMemorySizeInBytes();
+        assertThat(filtercacheSize, greaterThan(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"))
+                ))
+                .get();
+        assertHitCount(searchResponse, 1l);
+
+        // and because we use term filter, it is also added to filter cache, so it should contain more than before
+        statsResponse = client().admin().indices().prepareStats("test").clear().setFilterCache(true).get();
+        assertThat(statsResponse.getIndex("test").getTotal().getFilterCache().getMemorySizeInBytes(), greaterThan(filtercacheSize));
+        filtercacheSize = statsResponse.getIndex("test").getTotal().getFilterCache().getMemorySizeInBytes();
+
+        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 only has a cache entry for the term filter
+        // The range filter is now explicitly cached, so it now it is in the filter cache.
         statsResponse = client().admin().indices().prepareStats("test").clear().setFilterCache(true).get();
-        assertThat(statsResponse.getIndex("test").getTotal().getFilterCache().getMemorySizeInBytes(), greaterThan(0l));
+        assertThat(statsResponse.getIndex("test").getTotal().getFilterCache().getMemorySizeInBytes(), greaterThan(filtercacheSize));
     }
 
     @Test