Query DSL: Cache range filter on date field by default

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 elastic#7114.

(cherry picked from commit 9e68687)

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

@@ -329,19 +329,20 @@ 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, context, false);
+        return rangeFilter(lowerTerm, upperTerm, includeLower, includeUpper, context, null);
     }
 
-    public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context, boolean explicitCaching) {
-        boolean cache = explicitCaching;
+    public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @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);
             }
         }
@@ -350,14 +351,25 @@ public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLow
                 upperVal = ((Number) upperTerm).longValue();
             } else {
                 String value = convertToString(upperTerm);
-                cache = explicitCaching || !hasNowExpressionWithNoRounding(value);
+                cacheable = cacheable && !hasDateExpressionWithNoRounding(value);
                 upperVal = parseToMilliseconds(value, context, includeUpper);
             }
         }
 
+        if (explicitCaching != null) {
+            if (explicitCaching) {
+                cache = cacheable;
+            } else {
+                cache = false;
+            }
+        } else {
+            cache = cacheable;
+        }
+
         Filter filter =  NumericRangeFilter.newLongRange(
-            names.indexName(), precisionStep, lowerVal, upperVal, includeLower, includeUpper
+                names.indexName(), precisionStep, lowerVal, upperVal, includeLower, includeUpper
         );
+
         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.
@@ -369,45 +381,65 @@ public Filter rangeFilter(Object lowerTerm, Object upperTerm, boolean includeLow
 
     @Override
     public Filter rangeFilter(QueryParseContext parseContext, Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable QueryParseContext context) {
-        return rangeFilter(parseContext, lowerTerm, upperTerm, includeLower, includeUpper, context, false);
+        return rangeFilter(parseContext, lowerTerm, upperTerm, includeLower, includeUpper, context, null);
     }
 
-    public Filter rangeFilter(QueryParseContext parseContext, 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) {
-            if (lowerTerm instanceof Number) {
-                lowerVal = ((Number) lowerTerm).longValue();
+    public Filter rangeFilter(QueryParseContext parseContext, Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @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);
+                    cacheable = !hasDateExpressionWithNoRounding(value);
+                    lowerVal = parseToMilliseconds(value, context, false);
+                }
+            }
+            if (upperTerm != null) {
+                if (upperTerm instanceof Number) {
+                    upperVal = ((Number) upperTerm).longValue();
+                } else {
+                    String value = convertToString(upperTerm);
+                    cacheable = cacheable && !hasDateExpressionWithNoRounding(value);
+                    upperVal = parseToMilliseconds(value, context, includeUpper);
+                }
+            }
+
+            if (explicitCaching != null) {
+                if (explicitCaching) {
+                    cache = cacheable;
+                } else {
+                    cache = false;
+                }
             } else {
-                String value = convertToString(lowerTerm);
-                cache = explicitCaching || !hasNowExpressionWithNoRounding(value);
-                lowerVal = parseToMilliseconds(value, context, false);
+                cache = cacheable;
             }
-        }
-        if (upperTerm != null) {
-            if (upperTerm instanceof Number) {
-                upperVal = ((Number) upperTerm).longValue();
+
+            Filter filter;
+            if (parseContext != null) {
+                filter =  NumericRangeFieldDataFilter.newLongRange(
+                        (IndexNumericFieldData) parseContext.getForField(this), lowerVal,upperVal, includeLower, includeUpper
+                );
             } else {
-                String value = convertToString(upperTerm);
-                cache = explicitCaching || !hasNowExpressionWithNoRounding(value);
-                upperVal = parseToMilliseconds(value, context, includeUpper);
+                filter =  NumericRangeFilter.newLongRange(
+                        names.indexName(), precisionStep, lowerVal, upperVal, includeLower, includeUpper
+                );
             }
-        }
 
-        Filter filter =  NumericRangeFieldDataFilter.newLongRange(
-            (IndexNumericFieldData<?>) parseContext.getForField(this), lowerVal,upperVal, includeLower, includeUpper
-        );
-        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);
-        } else {
-            return filter;
+            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);
+            } else {
+                return filter;
+            }
         }
-    }
 
-    private boolean hasNowExpressionWithNoRounding(String value) {
+    private boolean hasDateExpressionWithNoRounding(String value) {
+
         int index = value.indexOf("now");
         if (index != -1) {
             if (value.length() == 3) {

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

@@ -120,10 +120,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;
@@ -160,8 +160,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);
@@ -170,4 +172,4 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
         }
         return filter;
     }
-}
+}

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

@@ -141,6 +141,118 @@ private BytesRef longToPrefixCoded(long val, int shift) {
     }
 
 
+    /**
+     * 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();
@@ -151,6 +263,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));

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

@@ -23,4 +23,4 @@
             }
         }
     }
-}
+}

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

@@ -0,0 +1,25 @@
+{
+    "constant_score": {
+        "filter": {
+            "bool": {
+                "must": [
+                    {
+                        "term": {
+                            "foo": {
+                                "value": "bar"
+                            }
+                        }
+                    },
+                    {
+                        "range" : {
+                            "born" : {
+                                "gte": 1325376000,
+                                "lte": 1577836800
+                            }
+                        }
+                    }
+                ]
+            }
+        }
+    }
+}

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

@@ -0,0 +1,26 @@
+{
+    "constant_score": {
+        "filter": {
+            "bool": {
+                "must": [
+                    {
+                        "term": {
+                            "foo": {
+                                "value": "bar"
+                            }
+                        }
+                    },
+                    {
+                        "range" : {
+                            "_cache" : false,
+                            "born" : {
+                                "gte": 1325376000,
+                                "lte": 1577836800
+                            }
+                        }
+                    }
+                ]
+            }
+        }
+    }
+}

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

@@ -2334,9 +2334,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(), greaterThan(filtercacheSize));
+        assertThat(statsResponse.getIndex("test").getTotal().getFilterCache().getMemorySizeInBytes(), is(filtercacheSize));
     }
 
     @Test