Make range query rounding consistent

docs/reference/migration/migrate_8_0/search.asciidoc

@@ -50,4 +50,5 @@ The vector functions of the form `function(query, doc['field'])` were
 deprecated in 7.6, and are now removed in 8.x. The form
 `function(query, 'field')` should be used instead. For example,
 `cosineSimilarity(query, doc['field'])` is replaced by
-`cosineSimilarity(query, 'field')`.
+`cosineSimilarity(query, 'field')`.
+

rest-api-spec/src/main/resources/rest-api-spec/test/range/10_basic.yml

@@ -391,3 +391,59 @@ setup:
         body: { "size" : 0, "query" : { "range" : { "date_range" : { "gte": "2017-09-03", "lte" : "2017-09-04", "relation": "within" } } } }
 
   - match: { hits.total: 0 }
+
+---
+"Date range rounding":
+  - skip:
+      version: " - 7.99.99"
+      reason:  "This part tests rounding behaviour changed in 8.0.0"
+
+  - do:
+      index:
+        index: test
+        id: 1
+        body: { "date_range" : { "gte": "2019-12-14T12:00:00.000Z", "lte": "2019-12-14T13:00:00.000Z" } }
+
+  - do:
+      index:
+        index: test
+        id: 2
+        body: { "date_range" : { "gte": "2019-12-15T12:00:00.000Z", "lte": "2019-12-15T13:00:00.000Z" } }
+
+  - do:
+      index:
+        index: test
+        id: 3
+        body: { "date_range" : { "gte": "2019-12-16T12:00:00.000Z", "lte": "2019-12-16T13:00:00.000Z" } }
+
+
+  - do:
+      indices.refresh: {}
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        body: { "size" : 0, "query" : { "range" : { "date_range" : { "gt": "2019-12-15||/d", "relation": "within" } } } }
+
+  - match: { hits.total: 1 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        body: { "size" : 0, "query" : { "range" : { "date_range" : { "gte": "2019-12-15||/d", "relation": "within" } } } }
+
+  - match: { hits.total: 2 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        body: { "size" : 0, "query" : { "range" : { "date_range" : { "lt": "2019-12-15||/d", "relation": "within" } } } }
+
+  - match: { hits.total: 1 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        body: { "size" : 0, "query" : { "range" : { "date_range" : { "lte": "2019-12-15||/d", "relation": "within" } } } }
+
+  - match: { hits.total: 2 }

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

@@ -63,6 +63,7 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
+import java.util.function.LongSupplier;
 
 import static org.elasticsearch.common.time.DateUtils.toLong;
 
@@ -371,15 +372,15 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
             if (lowerTerm == null) {
                 l = Long.MIN_VALUE;
             } else {
-                l = parseToLong(lowerTerm, !includeLower, timeZone, parser, context);
+                l = parseToLong(lowerTerm, !includeLower, timeZone, parser, context::nowInMillis);
                 if (includeLower == false) {
                     ++l;
                 }
             }
             if (upperTerm == null) {
                 u = Long.MAX_VALUE;
             } else {
-                u = parseToLong(upperTerm, includeUpper, timeZone, parser, context);
+                u = parseToLong(upperTerm, includeUpper, timeZone, parser, context::nowInMillis);
                 if (includeUpper == false) {
                     --u;
                 }
@@ -393,7 +394,7 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
         }
 
         public long parseToLong(Object value, boolean roundUp,
-                                @Nullable ZoneId zone, @Nullable DateMathParser forcedDateParser, QueryRewriteContext context) {
+                                @Nullable ZoneId zone, @Nullable DateMathParser forcedDateParser, LongSupplier now) {
             DateMathParser dateParser = dateMathParser();
             if (forcedDateParser != null) {
                 dateParser = forcedDateParser;
@@ -405,7 +406,7 @@ public long parseToLong(Object value, boolean roundUp,
             } else {
                 strValue = value.toString();
             }
-            Instant instant = dateParser.parse(strValue, context::nowInMillis, roundUp, zone);
+            Instant instant = dateParser.parse(strValue, now, roundUp, zone);
             return resolution.convert(instant);
         }
 
@@ -419,7 +420,7 @@ public Relation isFieldWithinQuery(IndexReader reader,
 
             long fromInclusive = Long.MIN_VALUE;
             if (from != null) {
-                fromInclusive = parseToLong(from, !includeLower, timeZone, dateParser, context);
+                fromInclusive = parseToLong(from, !includeLower, timeZone, dateParser, context::nowInMillis);
                 if (includeLower == false) {
                     if (fromInclusive == Long.MAX_VALUE) {
                         return Relation.DISJOINT;
@@ -430,7 +431,7 @@ public Relation isFieldWithinQuery(IndexReader reader,
 
             long toInclusive = Long.MAX_VALUE;
             if (to != null) {
-                toInclusive = parseToLong(to, includeUpper, timeZone, dateParser, context);
+                toInclusive = parseToLong(to, includeUpper, timeZone, dateParser, context::nowInMillis);
                 if (includeUpper == false) {
                     if (toInclusive == Long.MIN_VALUE) {
                         return Relation.DISJOINT;

server/src/main/java/org/elasticsearch/index/mapper/RangeType.java

@@ -232,12 +232,14 @@ public Query rangeQuery(String field, boolean hasDocValues, Object lowerTerm, Ob
 
             DateMathParser dateMathParser = (parser == null) ?
                 DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.toDateMathParser() : parser;
+            boolean roundUp = includeLower == false; // using "gt" should round lower bound up
             Long low = lowerTerm == null ? Long.MIN_VALUE :
                 dateMathParser.parse(lowerTerm instanceof BytesRef ? ((BytesRef) lowerTerm).utf8ToString() : lowerTerm.toString(),
-                    context::nowInMillis, false, zone).toEpochMilli();
+                    context::nowInMillis, roundUp, zone).toEpochMilli();
+            roundUp = includeUpper; // using "lte" should round upper bound up
             Long high = upperTerm == null ? Long.MAX_VALUE :
                 dateMathParser.parse(upperTerm instanceof BytesRef ? ((BytesRef) upperTerm).utf8ToString() : upperTerm.toString(),
-                    context::nowInMillis, false, zone).toEpochMilli();
+                    context::nowInMillis, roundUp, zone).toEpochMilli();
 
             return super.rangeQuery(field, hasDocValues, low, high, includeLower, includeUpper, relation, zone,
                 dateMathParser, context);

server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java

@@ -30,16 +30,15 @@
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.unit.DistanceUnit;
+import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.ObjectParser;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.common.unit.TimeValue;
-
 import org.elasticsearch.index.mapper.DateFieldMapper;
-import org.elasticsearch.index.mapper.MappedFieldType;
-import org.elasticsearch.index.mapper.GeoPointFieldMapper.GeoPointFieldType;
 import org.elasticsearch.index.mapper.DateFieldMapper.DateFieldType;
+import org.elasticsearch.index.mapper.GeoPointFieldMapper.GeoPointFieldType;
+import org.elasticsearch.index.mapper.MappedFieldType;
 
 import java.io.IOException;
 import java.util.Objects;
@@ -122,7 +121,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
         }
         Object originObj = origin.origin();
         if (fieldType instanceof DateFieldType) {
-            long originLong = ((DateFieldType) fieldType).parseToLong(originObj, true, null, null, context);
+            long originLong = ((DateFieldType) fieldType).parseToLong(originObj, true, null, null, context::nowInMillis);
             TimeValue pivotVal = TimeValue.parseTimeValue(pivot, DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot");
             if (((DateFieldType) fieldType).resolution() == DateFieldMapper.Resolution.MILLISECONDS) {
                 return LongPoint.newDistanceFeatureQuery(field, boost, originLong, pivotVal.getMillis());
@@ -213,7 +212,9 @@ Object origin() {
 
         @Override
         public final boolean equals(Object other) {
-            if ((other instanceof Origin) == false) return false;
+            if ((other instanceof Origin) == false) {
+                return false;
+            }
             Object otherOrigin = ((Origin) other).origin();
             return this.origin().equals(otherOrigin);
         }

server/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java

@@ -318,7 +318,7 @@ private AbstractDistanceScoreFunction parseDateVariable(XContentParser parser, Q
         if (originString == null) {
             origin = context.nowInMillis();
         } else {
-            origin = ((DateFieldMapper.DateFieldType) dateFieldType).parseToLong(originString, false, null, null, context);
+            origin = ((DateFieldMapper.DateFieldType) dateFieldType).parseToLong(originString, false, null, null, context::nowInMillis);
         }
 
         if (scaleString == null) {

server/src/test/java/org/elasticsearch/index/mapper/RangeFieldQueryStringQueryBuilderTests.java

@@ -23,7 +23,9 @@
 import org.apache.lucene.document.FloatRange;
 import org.apache.lucene.document.InetAddressRange;
 import org.apache.lucene.document.IntRange;
+import org.apache.lucene.document.LongPoint;
 import org.apache.lucene.document.LongRange;
+import org.apache.lucene.document.SortedNumericDocValuesField;
 import org.apache.lucene.queries.BinaryDocValuesRangeQuery;
 import org.apache.lucene.search.IndexOrDocValuesQuery;
 import org.apache.lucene.search.PointRangeQuery;
@@ -102,14 +104,29 @@ public void testDateRangeQuery() throws Exception {
         RangeFieldMapper.RangeFieldType type = (RangeFieldMapper.RangeFieldType) context.fieldMapper(DATE_RANGE_FIELD_NAME);
         DateMathParser parser = type.dateMathParser;
         Query query = new QueryStringQueryBuilder(DATE_RANGE_FIELD_NAME + ":[2010-01-01 TO 2018-01-01]").toQuery(createShardContext());
+        String lowerBoundExact = "2010-01-01T00:00:00.000";
+        String upperBoundExact = "2018-01-01T23:59:59.999";
         Query range = LongRange.newIntersectsQuery(DATE_RANGE_FIELD_NAME,
-            new long[]{ parser.parse("2010-01-01", () -> 0).toEpochMilli()},
-            new long[]{ parser.parse("2018-01-01", () -> 0).toEpochMilli()});
+            new long[]{ parser.parse(lowerBoundExact, () -> 0).toEpochMilli()},
+            new long[]{ parser.parse(upperBoundExact, () -> 0).toEpochMilli()});
         Query dv = RangeType.DATE.dvRangeQuery(DATE_RANGE_FIELD_NAME,
             BinaryDocValuesRangeQuery.QueryType.INTERSECTS,
-            parser.parse("2010-01-01", () -> 0).toEpochMilli(),
-            parser.parse("2018-01-01", () -> 0).toEpochMilli(), true, true);
+            parser.parse(lowerBoundExact, () -> 0).toEpochMilli(),
+            parser.parse(upperBoundExact, () -> 0).toEpochMilli(), true, true);
         assertEquals(new IndexOrDocValuesQuery(range, dv), query);
+
+        // also make sure the produced bounds are the same as on a regular `date` field
+        DateFieldMapper.DateFieldType dateType = (DateFieldMapper.DateFieldType) context.fieldMapper(DATE_FIELD_NAME);
+        parser = dateType.dateMathParser;
+        Query queryOnDateField = new QueryStringQueryBuilder(DATE_FIELD_NAME + ":[2010-01-01 TO 2018-01-01]").toQuery(createShardContext());
+        Query controlQuery = LongPoint.newRangeQuery(DATE_FIELD_NAME,
+                new long[]{ parser.parse(lowerBoundExact, () -> 0).toEpochMilli()},
+                new long[]{ parser.parse(upperBoundExact, () -> 0).toEpochMilli()});
+
+        Query controlDv = SortedNumericDocValuesField.newSlowRangeQuery(DATE_FIELD_NAME,
+                parser.parse(lowerBoundExact, () -> 0).toEpochMilli(),
+                parser.parse(upperBoundExact, () -> 0).toEpochMilli());
+        assertEquals(new IndexOrDocValuesQuery(controlQuery, controlDv), queryOnDateField);
     }
 
     public void testIPRangeQuery() throws Exception {

server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java

@@ -40,6 +40,7 @@
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.index.IndexSettings;
+import org.elasticsearch.index.mapper.DateFieldMapper.DateFieldType;
 import org.elasticsearch.index.mapper.RangeFieldMapper.RangeFieldType;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.test.IndexSettingsModule;
@@ -162,7 +163,7 @@ public void testRangeQueryIntersectsAdjacentValues() throws Exception {
             assertThat(rangeQuery, instanceOf(IndexOrDocValuesQuery.class));
             assertThat(((IndexOrDocValuesQuery) rangeQuery).getIndexQuery(), instanceOf(MatchNoDocsQuery.class));
     }
-    
+
     /**
      * check that we catch cases where the user specifies larger "from" than "to" value, not counting the include upper/lower settings
      */
@@ -231,14 +232,15 @@ private QueryShardContext createContext() {
         return new QueryShardContext(0, idxSettings, BigArrays.NON_RECYCLING_INSTANCE, null, null, null, null, null,
             xContentRegistry(), writableRegistry(), null, null, () -> nowInMillis, null, null);
     }
-    
+
     public void testDateRangeQueryUsingMappingFormat() {
         QueryShardContext context = createContext();
         RangeFieldType fieldType = new RangeFieldType(RangeType.DATE);
         fieldType.setName(FIELDNAME);
         fieldType.setIndexOptions(IndexOptions.DOCS);
         fieldType.setHasDocValues(false);
-        ShapeRelation relation = randomFrom(ShapeRelation.values());
+        // don't use DISJOINT here because it doesn't work on date fields which we want to compare bounds with
+        ShapeRelation relation = randomValueOtherThan(ShapeRelation.DISJOINT,() -> randomFrom(ShapeRelation.values()));
 
         // dates will break the default format, month/day of month is turned around in the format
         final String from = "2016-15-06T15:29:50+08:00";
@@ -257,7 +259,61 @@ public void testDateRangeQueryUsingMappingFormat() {
 
         fieldType.setDateTimeFormatter(formatter);
         final Query query = fieldType.rangeQuery(from, to, true, true, relation, null, null, context);
-        assertEquals("field:<ranges:[1465975790000 : 1466062190000]>", query.toString());
+        assertEquals("field:<ranges:[1465975790000 : 1466062190999]>", query.toString());
+
+        // compare lower and upper bounds with what we would get on a `date` field
+        DateFieldType dateFieldType = new DateFieldType();
+        dateFieldType.setName(FIELDNAME);
+        dateFieldType.setDateTimeFormatter(formatter);
+        final Query queryOnDateField = dateFieldType.rangeQuery(from, to, true, true, relation, null, null, context);
+        assertEquals("field:[1465975790000 TO 1466062190999]", queryOnDateField.toString());
+    }
+
+    /**
+     * We would like to ensure lower and upper bounds are consistent between queries on a `date` and a`date_range`
+     * field, so we randomize a few cases and compare the generated queries here
+     */
+    public void testDateVsDateRangeBounds() {
+        QueryShardContext context = createContext();
+        RangeFieldType fieldType = new RangeFieldType(RangeType.DATE);
+        fieldType.setName(FIELDNAME);
+        fieldType.setIndexOptions(IndexOptions.DOCS);
+        fieldType.setHasDocValues(false);
+
+        // date formatter that truncates seconds, so we get some rounding behavior
+        final DateFormatter formatter = DateFormatter.forPattern("yyyy-dd-MM'T'HH:mm");
+        long lower = randomLongBetween(formatter.parseMillis("2000-01-01T00:00"), formatter.parseMillis("2010-01-01T00:00"));
+        long upper = randomLongBetween(formatter.parseMillis("2011-01-01T00:00"), formatter.parseMillis("2020-01-01T00:00"));
+
+        fieldType.setDateTimeFormatter(formatter);
+        String lowerAsString = formatter.formatMillis(lower);
+        String upperAsString = formatter.formatMillis(upper);
+        // also add date math rounding to days occasionally
+        if (randomBoolean()) {
+            lowerAsString = lowerAsString + "||/d";
+        }
+        if (randomBoolean()) {
+            upperAsString = upperAsString + "||/d";
+        }
+        boolean includeLower = randomBoolean();
+        boolean includeUpper = randomBoolean();
+        final Query query = fieldType.rangeQuery(lowerAsString, upperAsString, includeLower, includeUpper, ShapeRelation.INTERSECTS, null,
+                null, context);
+
+        // get exact lower and upper bounds similar to what we would parse for `date` fields for same input strings
+        DateFieldType dateFieldType = new DateFieldType();
+        long lowerBoundLong = dateFieldType.parseToLong(lowerAsString, !includeLower, null, formatter.toDateMathParser(), () -> 0);
+        if (includeLower == false) {
+            ++lowerBoundLong;
+        }
+        long upperBoundLong = dateFieldType.parseToLong(upperAsString, includeUpper, null, formatter.toDateMathParser(), () -> 0);
+        if (includeUpper == false) {
+            --upperBoundLong;
+        }
+
+        // check that using this bounds we get similar query when constructing equivalent query on date_range field
+        Query range = LongRange.newIntersectsQuery(FIELDNAME, new long[] { lowerBoundLong }, new long[] { upperBoundLong });
+        assertEquals(range, query);
     }
 
     private Query getExpectedRangeQuery(ShapeRelation relation, Object from, Object to, boolean includeLower, boolean includeUpper) {

server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java

@@ -28,12 +28,11 @@
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.index.mapper.DateFieldMapper;
+import org.elasticsearch.index.mapper.DateFieldMapper.DateFieldType;
 import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.index.query.DistanceFeatureQueryBuilder.Origin;
 import org.elasticsearch.test.AbstractQueryTestCase;
 import org.joda.time.DateTime;
-import org.elasticsearch.index.query.DistanceFeatureQueryBuilder.Origin;
-import org.elasticsearch.index.mapper.DateFieldMapper.DateFieldType;
-
 
 import java.io.IOException;
 import java.time.Instant;
@@ -88,7 +87,7 @@ protected void doAssertLuceneQuery(DistanceFeatureQueryBuilder queryBuilder,
         } else { // if (fieldName.equals(DATE_FIELD_NAME))
             MapperService mapperService = context.getMapperService();
             DateFieldType fieldType = (DateFieldType) mapperService.fullName(fieldName);
-            long originLong = fieldType.parseToLong(origin, true, null, null, context);
+            long originLong = fieldType.parseToLong(origin, true, null, null, context::nowInMillis);
             TimeValue pivotVal = TimeValue.parseTimeValue(pivot, DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot");
             long pivotLong;
             if (fieldType.resolution() == DateFieldMapper.Resolution.MILLISECONDS) {

server/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java

@@ -170,12 +170,12 @@ protected void doAssertLuceneQuery(RangeQueryBuilder queryBuilder, Query query,
                     ((DateFieldMapper.DateFieldType) mappedFieldType).parseToLong(queryBuilder.from(),
                         queryBuilder.includeLower(),
                         queryBuilder.getDateTimeZone(),
-                        queryBuilder.getForceDateParser(), context);
+                        queryBuilder.getForceDateParser(), context::nowInMillis);
                 toInMillis = queryBuilder.to() == null ? null :
                     ((DateFieldMapper.DateFieldType) mappedFieldType).parseToLong(queryBuilder.to(),
                         queryBuilder.includeUpper(),
                         queryBuilder.getDateTimeZone(),
-                        queryBuilder.getForceDateParser(), context);
+                        queryBuilder.getForceDateParser(), context::nowInMillis);
             } else {
                 fromInMillis = toInMillis = null;
                 fail("unexpected mapped field type: [" + mappedFieldType.getClass() + "] " + mappedFieldType.toString());