Browse Source

Fix bucket keys format for range aggregations on float field (#81801)

When dealing with float fields we actually store float values with lower precision.
Later on, when loading float values into double values, the precision difference between
float and double values surfaces as additional "spurious" digits in the string
representation which is propagated to the client during serialisation. As a result of this,
the JSON response returned to the client includes range values not matching values in the
request. This results in clients, including Kibana, to break while trying to match ranges in
the request with ranges in the response.

With this change we use two new fields, `originalFrom` and `originalTo` to hold the original
double values before manipulating the precision of float fields. Later we use these fields in
the code dealing with serialisation. The effect is that precision issue resulting from
float to double conversions are not propagated to clients.
Salvatore Campagna 3 years ago
parent
commit
3550370c86

+ 76 - 0
rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/40_range.yml

@@ -91,8 +91,84 @@ setup:
   - match: { aggregations.double_range.buckets.1.key: "0.0152-1.0" }
   - match: { aggregations.double_range.buckets.1.doc_count: 2 }
 
+---
+"Float range":
+  - skip:
+      version: " - 8.0.99"
+      reason: Bug fixed in 8.1.0
+  - do:
+      search:
+        body:
+          size: 0
+          aggs:
+            float_range:
+              range:
+                field: "float"
+                ranges:
+                  -
+                    to: 6.0
+                  -
+                    from: 6.0
+                    to: 10.6
+                  -
+                    from: 10.6
+
+  - match: { hits.total.relation: "eq" }
+  - match: { hits.total.value: 4 }
+  - length: { aggregations.float_range.buckets: 3 }
+  - match: { aggregations.float_range.buckets.0.key: "*-6.0" }
+  - is_false: aggregations.float_range.buckets.0.from
+  - match: { aggregations.float_range.buckets.0.to: 6.0 }
+  - match: { aggregations.float_range.buckets.0.doc_count: 3 }
+  - match: { aggregations.float_range.buckets.1.key: "6.0-10.6" }
+  - match: { aggregations.float_range.buckets.1.from: 6.0 }
+  - match: { aggregations.float_range.buckets.1.to: 10.6 }
+  - match: { aggregations.float_range.buckets.1.doc_count: 0 }
+  - match: { aggregations.float_range.buckets.2.key: "10.6-*" }
+  - match: { aggregations.float_range.buckets.2.from: 10.6 }
+  - is_false:  aggregations.float_range.buckets.2.to
+  - match: { aggregations.float_range.buckets.2.doc_count: 0 }
+
 ---
 "Double range":
+  - skip:
+      version: " - 8.0.99"
+      reason: Bug fixed in 8.1.0
+  - do:
+      search:
+        body:
+          size: 0
+          aggs:
+            float_range:
+              range:
+                field: "double"
+                ranges:
+                  -
+                    to: 6.0
+                  -
+                    from: 6.0
+                    to: 10.6
+                  -
+                    from: 10.6
+
+  - match: { hits.total.relation: "eq" }
+  - match: { hits.total.value: 4 }
+  - length: { aggregations.float_range.buckets: 3 }
+  - match: { aggregations.float_range.buckets.0.key: "*-6.0" }
+  - is_false: aggregations.float_range.buckets.0.from
+  - match: { aggregations.float_range.buckets.0.to: 6.0 }
+  - match: { aggregations.float_range.buckets.0.doc_count: 0 }
+  - match: { aggregations.float_range.buckets.1.key: "6.0-10.6" }
+  - match: { aggregations.float_range.buckets.1.from: 6.0 }
+  - match: { aggregations.float_range.buckets.1.to: 10.6 }
+  - match: { aggregations.float_range.buckets.1.doc_count: 0 }
+  - match: { aggregations.float_range.buckets.2.key: "10.6-*" }
+  - match: { aggregations.float_range.buckets.2.from: 10.6 }
+  - is_false:  aggregations.float_range.buckets.2.to
+  - match: { aggregations.float_range.buckets.2.doc_count: 3 }
+
+---
+"Double range no decimal":
   - do:
       search:
         body:

+ 1 - 1
server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/DateRangeAggregationBuilder.java

@@ -364,7 +364,7 @@ public class DateRangeAggregationBuilder extends AbstractRangeBuilder<DateRangeA
             } else if (Double.isFinite(to)) {
                 to = parser.parseDouble(Long.toString((long) to), false, context::nowInMillis);
             }
-            return new RangeAggregator.Range(range.getKey(), from, fromAsString, to, toAsString);
+            return new RangeAggregator.Range(range.getKey(), from, from, fromAsString, to, to, toAsString);
         });
         if (ranges.length == 0) {
             throw new IllegalArgumentException("No [ranges] specified for the [" + this.getName() + "] aggregation");

+ 19 - 3
server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalDateRange.java

@@ -28,25 +28,29 @@ public class InternalDateRange extends InternalRange<InternalDateRange.Bucket, I
         public Bucket(
             String key,
             double from,
+            double originalFrom,
             double to,
+            double originalTo,
             long docCount,
             List<InternalAggregation> aggregations,
             boolean keyed,
             DocValueFormat formatter
         ) {
-            super(key, from, to, docCount, InternalAggregations.from(aggregations), keyed, formatter);
+            super(key, from, originalFrom, to, originalTo, docCount, InternalAggregations.from(aggregations), keyed, formatter);
         }
 
         public Bucket(
             String key,
             double from,
+            double originalFrom,
             double to,
+            double originalTo,
             long docCount,
             InternalAggregations aggregations,
             boolean keyed,
             DocValueFormat formatter
         ) {
-            super(key, from, to, docCount, aggregations, keyed, formatter);
+            super(key, from, originalFrom, to, originalTo, docCount, aggregations, keyed, formatter);
         }
 
         @Override
@@ -71,6 +75,14 @@ public class InternalDateRange extends InternalRange<InternalDateRange.Bucket, I
             return to;
         }
 
+        private Double internalGetOriginalFrom() {
+            return originalFrom;
+        }
+
+        private Double internalGetOriginalTo() {
+            return originalTo;
+        }
+
         @Override
         protected InternalRange.Factory<Bucket, ?> getFactory() {
             return FACTORY;
@@ -112,13 +124,15 @@ public class InternalDateRange extends InternalRange<InternalDateRange.Bucket, I
         public Bucket createBucket(
             String key,
             double from,
+            double originalFrom,
             double to,
+            double originalTo,
             long docCount,
             InternalAggregations aggregations,
             boolean keyed,
             DocValueFormat formatter
         ) {
-            return new Bucket(key, from, to, docCount, aggregations, keyed, formatter);
+            return new Bucket(key, from, originalFrom, to, originalTo, docCount, aggregations, keyed, formatter);
         }
 
         @Override
@@ -126,7 +140,9 @@ public class InternalDateRange extends InternalRange<InternalDateRange.Bucket, I
             return new Bucket(
                 prototype.getKey(),
                 prototype.internalGetFrom(),
+                prototype.internalGetOriginalFrom(),
                 prototype.internalGetTo(),
+                prototype.internalGetOriginalTo(),
                 prototype.getDocCount(),
                 aggregations,
                 prototype.getKeyed(),

+ 16 - 3
server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalGeoDistance.java

@@ -23,8 +23,17 @@ public class InternalGeoDistance extends InternalRange<InternalGeoDistance.Bucke
 
     static class Bucket extends InternalRange.Bucket {
 
-        Bucket(String key, double from, double to, long docCount, InternalAggregations aggregations, boolean keyed) {
-            super(key, from, to, docCount, aggregations, keyed, DocValueFormat.RAW);
+        Bucket(
+            String key,
+            double from,
+            double originalFrom,
+            double to,
+            double originalTo,
+            long docCount,
+            InternalAggregations aggregations,
+            boolean keyed
+        ) {
+            super(key, from, originalFrom, to, originalTo, docCount, aggregations, keyed, DocValueFormat.RAW);
         }
 
         @Override
@@ -68,13 +77,15 @@ public class InternalGeoDistance extends InternalRange<InternalGeoDistance.Bucke
         public Bucket createBucket(
             String key,
             double from,
+            double originalFrom,
             double to,
+            double originalTo,
             long docCount,
             InternalAggregations aggregations,
             boolean keyed,
             DocValueFormat format
         ) {
-            return new Bucket(key, from, to, docCount, aggregations, keyed);
+            return new Bucket(key, from, originalFrom, to, originalTo, docCount, aggregations, keyed);
         }
 
         @Override
@@ -82,7 +93,9 @@ public class InternalGeoDistance extends InternalRange<InternalGeoDistance.Bucke
             return new Bucket(
                 prototype.getKey(),
                 ((Number) prototype.getFrom()).doubleValue(),
+                ((Number) prototype.getOriginalFrom()).doubleValue(),
                 ((Number) prototype.getTo()).doubleValue(),
+                ((Number) prototype.getOriginalTo()).doubleValue(),
                 prototype.getDocCount(),
                 aggregations,
                 prototype.getKeyed()

+ 58 - 16
server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java

@@ -7,6 +7,7 @@
  */
 package org.elasticsearch.search.aggregations.bucket.range;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.search.DocValueFormat;
@@ -36,7 +37,9 @@ public class InternalRange<B extends InternalRange.Bucket, R extends InternalRan
         protected final transient boolean keyed;
         protected final transient DocValueFormat format;
         protected final double from;
+        protected final double originalFrom;
         protected final double to;
+        protected final double originalTo;
         private final long docCount;
         private final InternalAggregations aggregations;
         private final String key;
@@ -44,7 +47,9 @@ public class InternalRange<B extends InternalRange.Bucket, R extends InternalRan
         public Bucket(
             String key,
             double from,
+            double originalFrom,
             double to,
+            double originalTo,
             long docCount,
             InternalAggregations aggregations,
             boolean keyed,
@@ -52,9 +57,11 @@ public class InternalRange<B extends InternalRange.Bucket, R extends InternalRan
         ) {
             this.keyed = keyed;
             this.format = format;
-            this.key = key != null ? key : generateKey(from, to, format);
+            this.key = key != null ? key : generateKey(originalFrom, originalTo, format);
             this.from = from;
+            this.originalFrom = originalFrom;
             this.to = to;
+            this.originalTo = originalTo;
             this.docCount = docCount;
             this.aggregations = aggregations;
         }
@@ -74,11 +81,19 @@ public class InternalRange<B extends InternalRange.Bucket, R extends InternalRan
             return from;
         }
 
+        public Double getOriginalFrom() {
+            return originalFrom;
+        }
+
         @Override
         public Object getTo() {
             return to;
         }
 
+        public Double getOriginalTo() {
+            return originalTo;
+        }
+
         public boolean getKeyed() {
             return keyed;
         }
@@ -89,19 +104,19 @@ public class InternalRange<B extends InternalRange.Bucket, R extends InternalRan
 
         @Override
         public String getFromAsString() {
-            if (Double.isInfinite(from)) {
+            if (Double.isInfinite(originalFrom)) {
                 return null;
             } else {
-                return format.format(from).toString();
+                return format.format(originalFrom).toString();
             }
         }
 
         @Override
         public String getToAsString() {
-            if (Double.isInfinite(to)) {
+            if (Double.isInfinite(originalTo)) {
                 return null;
             } else {
-                return format.format(to).toString();
+                return format.format(originalTo).toString();
             }
         }
 
@@ -128,16 +143,16 @@ public class InternalRange<B extends InternalRange.Bucket, R extends InternalRan
                 builder.startObject();
                 builder.field(CommonFields.KEY.getPreferredName(), key);
             }
-            if (Double.isInfinite(from) == false) {
-                builder.field(CommonFields.FROM.getPreferredName(), from);
+            if (Double.isInfinite(originalFrom) == false) {
+                builder.field(CommonFields.FROM.getPreferredName(), originalFrom);
                 if (format != DocValueFormat.RAW) {
-                    builder.field(CommonFields.FROM_AS_STRING.getPreferredName(), format.format(from));
+                    builder.field(CommonFields.FROM_AS_STRING.getPreferredName(), format.format(originalFrom));
                 }
             }
-            if (Double.isInfinite(to) == false) {
-                builder.field(CommonFields.TO.getPreferredName(), to);
+            if (Double.isInfinite(originalTo) == false) {
+                builder.field(CommonFields.TO.getPreferredName(), originalTo);
                 if (format != DocValueFormat.RAW) {
-                    builder.field(CommonFields.TO_AS_STRING.getPreferredName(), format.format(to));
+                    builder.field(CommonFields.TO_AS_STRING.getPreferredName(), format.format(originalTo));
                 }
             }
             builder.field(CommonFields.DOC_COUNT.getPreferredName(), docCount);
@@ -157,7 +172,13 @@ public class InternalRange<B extends InternalRange.Bucket, R extends InternalRan
         public void writeTo(StreamOutput out) throws IOException {
             out.writeString(key);
             out.writeDouble(from);
+            if (out.getVersion().onOrAfter(Version.V_8_1_0)) {
+                out.writeOptionalDouble(originalFrom);
+            }
             out.writeDouble(to);
+            if (out.getVersion().onOrAfter(Version.V_8_1_0)) {
+                out.writeOptionalDouble(originalTo);
+            }
             out.writeVLong(docCount);
             aggregations.writeTo(out);
         }
@@ -202,13 +223,15 @@ public class InternalRange<B extends InternalRange.Bucket, R extends InternalRan
         public B createBucket(
             String key,
             double from,
+            double originalFrom,
             double to,
+            double originalTo,
             long docCount,
             InternalAggregations aggregations,
             boolean keyed,
             DocValueFormat format
         ) {
-            return (B) new Bucket(key, from, to, docCount, aggregations, keyed, format);
+            return (B) new Bucket(key, from, originalFrom, to, originalTo, docCount, aggregations, keyed, format);
         }
 
         @SuppressWarnings("unchecked")
@@ -221,7 +244,9 @@ public class InternalRange<B extends InternalRange.Bucket, R extends InternalRan
             return (B) new Bucket(
                 prototype.getKey(),
                 prototype.from,
+                prototype.originalFrom,
                 prototype.to,
+                prototype.originalTo,
                 prototype.getDocCount(),
                 aggregations,
                 prototype.keyed,
@@ -252,12 +277,19 @@ public class InternalRange<B extends InternalRange.Bucket, R extends InternalRan
         List<B> ranges = new ArrayList<>(size);
         for (int i = 0; i < size; i++) {
             String key = in.readString();
+            double from = in.readDouble();
+            Double originalFrom = in.getVersion().onOrAfter(Version.V_8_1_0) ? in.readOptionalDouble() : Double.valueOf(from);
+            double to = in.readDouble();
+            Double originalTo = in.getVersion().onOrAfter(Version.V_8_1_0) ? in.readOptionalDouble() : Double.valueOf(to);
+            long docCount = in.readVLong();
             ranges.add(
                 getFactory().createBucket(
                     key,
-                    in.readDouble(),
-                    in.readDouble(),
-                    in.readVLong(),
+                    from,
+                    originalFrom,
+                    to,
+                    originalTo,
+                    docCount,
                     InternalAggregations.readFrom(in),
                     keyed,
                     format
@@ -338,7 +370,17 @@ public class InternalRange<B extends InternalRange.Bucket, R extends InternalRan
         }
         final InternalAggregations aggs = InternalAggregations.reduce(aggregationsList, context);
         Bucket prototype = buckets.get(0);
-        return getFactory().createBucket(prototype.key, prototype.from, prototype.to, docCount, aggs, keyed, format);
+        return getFactory().createBucket(
+            prototype.key,
+            prototype.from,
+            prototype.originalFrom,
+            prototype.to,
+            prototype.originalTo,
+            docCount,
+            aggs,
+            keyed,
+            format
+        );
     }
 
     @Override

+ 9 - 1
server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregationBuilder.java

@@ -171,7 +171,8 @@ public class RangeAggregationBuilder extends AbstractRangeBuilder<RangeAggregati
             if (range.toAsStr != null) {
                 to = parser.parseDouble(range.toAsStr, false, context::nowInMillis);
             }
-            return new Range(range.key, from, range.fromAsStr, to, range.toAsStr);
+            String key = range.key != null ? range.key : generateKey(range.originalFrom, range.originalTo, config.format());
+            return new Range(key, from, range.from, range.fromAsStr, to, range.to, range.toAsStr);
         });
         if (ranges.length == 0) {
             throw new IllegalArgumentException("No [ranges] specified for the [" + this.getName() + "] aggregation");
@@ -200,4 +201,11 @@ public class RangeAggregationBuilder extends AbstractRangeBuilder<RangeAggregati
     protected ValuesSourceRegistry.RegistryKey<?> getRegistryKey() {
         return REGISTRY_KEY;
     }
+
+    private static String generateKey(double from, double to, DocValueFormat format) {
+        StringBuilder builder = new StringBuilder().append(Double.isInfinite(from) ? "*" : format.format(from))
+            .append("-")
+            .append(Double.isInfinite(to) ? "*" : format.format(to));
+        return builder.toString();
+    }
 }

+ 64 - 11
server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregator.java

@@ -10,6 +10,7 @@ package org.elasticsearch.search.aggregations.bucket.range;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.ScoreMode;
 import org.apache.lucene.search.ScorerSupplier;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
@@ -22,6 +23,7 @@ import org.elasticsearch.search.DocValueFormat;
 import org.elasticsearch.search.aggregations.AdaptingAggregator;
 import org.elasticsearch.search.aggregations.Aggregator;
 import org.elasticsearch.search.aggregations.AggregatorFactories;
+import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.CardinalityUpperBound;
 import org.elasticsearch.search.aggregations.InternalAggregation;
 import org.elasticsearch.search.aggregations.InternalAggregations;
@@ -95,8 +97,10 @@ public abstract class RangeAggregator extends BucketsAggregator {
 
         protected final String key;
         protected final double from;
+        protected final Double originalFrom;
         protected final String fromAsStr;
         protected final double to;
+        protected final Double originalTo;
         protected final String toAsStr;
 
         /**
@@ -107,21 +111,32 @@ public abstract class RangeAggregator extends BucketsAggregator {
          * {@code from} and {@code to} parameters if they are non-null
          * and finite. Otherwise they parse from {@code fromrStr} and
          * {@code toStr}.
+         * {@code originalFrom} and {@code originalTo} are used to preserve
+         * the original values of {@code from} and {@code to}. This is because
+         * precision is downgraded to float values when they are stored.
+         * Downgrading precision for float values surfaces into precision
+         * artifacts at serialization time as a result of treating float values
+         * as double values.
+         * (See {@link RangeAggregationBuilder#innerBuild(
+         * AggregationContext, ValuesSourceConfig, AggregatorFactory, AggregatorFactories.Builder
+         * )})
          */
-        public Range(String key, Double from, String fromAsStr, Double to, String toAsStr) {
+        public Range(String key, Double from, Double originalFrom, String fromAsStr, Double to, Double originalTo, String toAsStr) {
             this.key = key;
             this.from = from == null ? Double.NEGATIVE_INFINITY : from;
+            this.originalFrom = originalFrom == null ? Double.NEGATIVE_INFINITY : originalFrom;
             this.fromAsStr = fromAsStr;
             this.to = to == null ? Double.POSITIVE_INFINITY : to;
+            this.originalTo = originalTo == null ? Double.POSITIVE_INFINITY : originalTo;
             this.toAsStr = toAsStr;
         }
 
         public Range(String key, Double from, Double to) {
-            this(key, from, null, to, null);
+            this(key, from, from, null, to, to, null);
         }
 
         public Range(String key, String from, String to) {
-            this(key, null, from, null, to);
+            this(key, null, null, from, null, null, to);
         }
 
         /**
@@ -133,6 +148,8 @@ public abstract class RangeAggregator extends BucketsAggregator {
             toAsStr = in.readOptionalString();
             from = in.readDouble();
             to = in.readDouble();
+            originalFrom = in.getVersion().onOrAfter(Version.V_8_1_0) ? in.readOptionalDouble() : Double.valueOf(from);
+            originalTo = in.getVersion().onOrAfter(Version.V_8_1_0) ? in.readOptionalDouble() : Double.valueOf(to);
         }
 
         @Override
@@ -142,6 +159,10 @@ public abstract class RangeAggregator extends BucketsAggregator {
             out.writeOptionalString(toAsStr);
             out.writeDouble(from);
             out.writeDouble(to);
+            if (out.getVersion().onOrAfter(Version.V_8_1_0)) {
+                out.writeOptionalDouble(originalFrom);
+                out.writeOptionalDouble(originalTo);
+            }
         }
 
         public double getFrom() {
@@ -152,6 +173,14 @@ public abstract class RangeAggregator extends BucketsAggregator {
             return this.to;
         }
 
+        public Double getOriginalFrom() {
+            return originalFrom;
+        }
+
+        public Double getOriginalTo() {
+            return originalTo;
+        }
+
         public String getFromAsString() {
             return this.fromAsStr;
         }
@@ -179,11 +208,11 @@ public abstract class RangeAggregator extends BucketsAggregator {
             if (key != null) {
                 builder.field(KEY_FIELD.getPreferredName(), key);
             }
-            if (Double.isFinite(from)) {
-                builder.field(FROM_FIELD.getPreferredName(), from);
+            if (Double.isFinite(originalFrom)) {
+                builder.field(FROM_FIELD.getPreferredName(), originalFrom);
             }
-            if (Double.isFinite(to)) {
-                builder.field(TO_FIELD.getPreferredName(), to);
+            if (Double.isFinite(originalTo)) {
+                builder.field(TO_FIELD.getPreferredName(), originalTo);
             }
             if (fromAsStr != null) {
                 builder.field(FROM_FIELD.getPreferredName(), fromAsStr);
@@ -203,7 +232,7 @@ public abstract class RangeAggregator extends BucketsAggregator {
             Double toDouble = to instanceof Number ? ((Number) to).doubleValue() : null;
             String fromStr = from instanceof String ? (String) from : null;
             String toStr = to instanceof String ? (String) to : null;
-            return new Range(key, fromDouble, fromStr, toDouble, toStr);
+            return new Range(key, fromDouble, fromDouble, fromStr, toDouble, toDouble, toStr);
         });
 
         static {
@@ -481,7 +510,17 @@ public abstract class RangeAggregator extends BucketsAggregator {
             ranges.length,
             (offsetInOwningOrd, docCount, subAggregationResults) -> {
                 Range range = ranges[offsetInOwningOrd];
-                return rangeFactory.createBucket(range.key, range.from, range.to, docCount, subAggregationResults, keyed, format);
+                return rangeFactory.createBucket(
+                    range.key,
+                    range.from,
+                    range.originalFrom,
+                    range.to,
+                    range.originalTo,
+                    docCount,
+                    subAggregationResults,
+                    keyed,
+                    format
+                );
             },
             buckets -> rangeFactory.create(name, buckets, format, keyed, metadata())
         );
@@ -497,7 +536,9 @@ public abstract class RangeAggregator extends BucketsAggregator {
             org.elasticsearch.search.aggregations.bucket.range.Range.Bucket bucket = rangeFactory.createBucket(
                 range.key,
                 range.from,
+                range.originalFrom,
                 range.to,
+                range.originalTo,
                 0,
                 subAggs,
                 keyed,
@@ -548,7 +589,9 @@ public abstract class RangeAggregator extends BucketsAggregator {
             InternalAggregations subAggs = buildEmptySubAggregations();
             List<org.elasticsearch.search.aggregations.bucket.range.Range.Bucket> buckets = new ArrayList<>(ranges.length);
             for (RangeAggregator.Range range : ranges) {
-                buckets.add(factory.createBucket(range.key, range.from, range.to, 0, subAggs, keyed, format));
+                buckets.add(
+                    factory.createBucket(range.key, range.from, range.originalFrom, range.to, range.originalTo, 0, subAggs, keyed, format)
+                );
             }
             return factory.create(name, buckets, format, keyed, metadata());
         }
@@ -786,7 +829,17 @@ public abstract class RangeAggregator extends BucketsAggregator {
                 Range r = ranges[i];
                 InternalFilters.InternalBucket b = filters.getBuckets().get(i);
                 buckets.add(
-                    rangeFactory.createBucket(r.getKey(), r.getFrom(), r.getTo(), b.getDocCount(), b.getAggregations(), keyed, format)
+                    rangeFactory.createBucket(
+                        r.getKey(),
+                        r.getFrom(),
+                        r.getOriginalFrom(),
+                        r.getTo(),
+                        r.getOriginalTo(),
+                        b.getDocCount(),
+                        b.getAggregations(),
+                        keyed,
+                        format
+                    )
                 );
             }
             return rangeFactory.create(name(), buckets, format, keyed, filters.getMetadata());

+ 4 - 2
server/src/test/java/org/elasticsearch/action/search/SearchResponseMergerTests.java

@@ -471,8 +471,10 @@ public class SearchResponseMergerTests extends ESTestCase {
             totalCount += count;
             InternalDateRange.Bucket bucket = factory.createBucket(
                 "bucket",
-                0,
-                10000,
+                0D,
+                0D,
+                10000D,
+                10000D,
                 count,
                 InternalAggregations.EMPTY,
                 false,

+ 29 - 0
server/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/DateRangeHistogramAggregatorTests.java

@@ -61,6 +61,35 @@ public class DateRangeHistogramAggregatorTests extends AggregatorTestCase {
         );
     }
 
+    public void testKeys() throws Exception {
+        RangeFieldMapper.Range range = new RangeFieldMapper.Range(
+            RangeType.DATE,
+            asLong("2019-08-01T14:35:00"),
+            asLong("2019-08-05T15:33:21"),
+            true,
+            true
+        );
+        String[] expectedBucketKeys = {
+            "2019-08-01T00:00:00.000Z",
+            "2019-08-02T00:00:00.000Z",
+            "2019-08-03T00:00:00.000Z",
+            "2019-08-04T00:00:00.000Z",
+            "2019-08-05T00:00:00.000Z" };
+        testCase(
+            new MatchAllDocsQuery(),
+            builder -> builder.calendarInterval(DateHistogramInterval.DAY),
+            writer -> writer.addDocument(singleton(new BinaryDocValuesField(FIELD_NAME, RangeType.DATE.encodeRanges(singleton(range))))),
+            histo -> {
+                assertEquals(5, histo.getBuckets().size());
+                assertArrayEquals(
+                    expectedBucketKeys,
+                    histo.getBuckets().stream().map(InternalDateHistogram.Bucket::getKeyAsString).toArray()
+                );
+                assertTrue(AggregationInspectionHelper.hasValue(histo));
+            }
+        );
+    }
+
     public void testFormat() throws Exception {
         RangeFieldMapper.Range range = new RangeFieldMapper.Range(
             RangeType.DATE,

+ 5 - 2
server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/InternalDateRangeTests.java

@@ -80,7 +80,7 @@ public class InternalDateRangeTests extends InternalRangeTestCase<InternalDateRa
             int docCount = randomIntBetween(0, 1000);
             double from = range.v1();
             double to = range.v2();
-            buckets.add(new InternalDateRange.Bucket("range_" + i, from, to, docCount, aggregations, keyed, format));
+            buckets.add(new InternalDateRange.Bucket("range_" + i, from, from, to, to, docCount, aggregations, keyed, format));
         }
         return new InternalDateRange(name, buckets, format, keyed, metadata);
     }
@@ -113,11 +113,14 @@ public class InternalDateRangeTests extends InternalRangeTestCase<InternalDateRa
             case 2 -> {
                 buckets = new ArrayList<>(buckets);
                 double from = randomDouble();
+                double to = from + randomDouble();
                 buckets.add(
                     new InternalDateRange.Bucket(
                         "range_a",
                         from,
-                        from + randomDouble(),
+                        from,
+                        to,
+                        to,
                         randomNonNegativeLong(),
                         InternalAggregations.EMPTY,
                         false,

+ 5 - 2
server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/InternalGeoDistanceTests.java

@@ -62,7 +62,7 @@ public class InternalGeoDistanceTests extends InternalRangeTestCase<InternalGeoD
             int docCount = randomIntBetween(0, 1000);
             double from = range.v1();
             double to = range.v2();
-            buckets.add(new InternalGeoDistance.Bucket("range_" + i, from, to, docCount, aggregations, keyed));
+            buckets.add(new InternalGeoDistance.Bucket("range_" + i, from, from, to, to, docCount, aggregations, keyed));
         }
         return new InternalGeoDistance(name, buckets, keyed, metadata);
     }
@@ -94,11 +94,14 @@ public class InternalGeoDistanceTests extends InternalRangeTestCase<InternalGeoD
             case 2 -> {
                 buckets = new ArrayList<>(buckets);
                 double from = randomDouble();
+                double to = from + randomDouble();
                 buckets.add(
                     new InternalGeoDistance.Bucket(
                         "range_a",
                         from,
-                        from + randomDouble(),
+                        from,
+                        to,
+                        to,
                         randomNonNegativeLong(),
                         InternalAggregations.EMPTY,
                         false

+ 5 - 2
server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/InternalRangeTests.java

@@ -75,7 +75,7 @@ public class InternalRangeTests extends InternalRangeTestCase<InternalRange<Inte
             int docCount = randomIntBetween(0, 1000);
             double from = range.v1();
             double to = range.v2();
-            buckets.add(new InternalRange.Bucket("range_" + i, from, to, docCount, aggregations, keyed, format));
+            buckets.add(new InternalRange.Bucket("range_" + i, from, from, to, to, docCount, aggregations, keyed, format));
         }
         return new InternalRange<>(name, buckets, format, keyed, metadata);
     }
@@ -108,11 +108,14 @@ public class InternalRangeTests extends InternalRangeTestCase<InternalRange<Inte
             case 2 -> {
                 buckets = new ArrayList<>(buckets);
                 double from = randomDouble();
+                double to = from + randomDouble();
                 buckets.add(
                     new InternalRange.Bucket(
                         "range_a",
                         from,
-                        from + randomDouble(),
+                        from,
+                        to,
+                        to,
                         randomNonNegativeLong(),
                         InternalAggregations.EMPTY,
                         false,

+ 65 - 0
server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorTests.java

@@ -121,6 +121,71 @@ public class RangeAggregatorTests extends AggregatorTestCase {
         );
     }
 
+    public void testFloatRangeKeys() throws IOException {
+        final String fieldName = "test";
+        MappedFieldType field = new NumberFieldMapper.NumberFieldType(fieldName, NumberType.FLOAT);
+        testCase(
+            new RangeAggregationBuilder("0").field(fieldName).addRange(5, 6).addRange(6, 10.6).keyed(true),
+            new MatchAllDocsQuery(),
+            iw -> {
+                iw.addDocument(singleton(new SortedNumericDocValuesField(fieldName, NumericUtils.floatToSortableInt(10))));
+                iw.addDocument(singleton(new SortedNumericDocValuesField(fieldName, NumericUtils.floatToSortableInt(5.5F))));
+                iw.addDocument(singleton(new SortedNumericDocValuesField(fieldName, NumericUtils.floatToSortableInt(6.7F))));
+            },
+            result -> {
+                InternalRange<?, ?> range = (InternalRange<?, ?>) result;
+                List<? extends InternalRange.Bucket> ranges = range.getBuckets();
+                assertEquals(2, ranges.size());
+                assertEquals("5.0-6.0", ranges.get(0).getKeyAsString());
+                assertEquals("6.0-10.6", ranges.get(1).getKeyAsString());
+                assertEquals("5.0", ranges.get(0).getFromAsString());
+                assertEquals("6.0", ranges.get(0).getToAsString());
+                assertEquals("6.0", ranges.get(1).getFromAsString());
+                assertEquals("10.6", ranges.get(1).getToAsString());
+                // NOTE: `getOriginalFrom` and `getOriginalTo` return double
+                assertEquals(5.0D, ranges.get(0).getOriginalFrom(), 0.0000000000001);
+                assertEquals(6.0D, ranges.get(0).getOriginalTo(), 0.0000000000001);
+                assertEquals(6.0D, ranges.get(1).getOriginalFrom(), 0.0000000000001);
+                assertEquals(10.6D, ranges.get(1).getOriginalTo(), 0.0000000000001);
+                assertEquals(1, ranges.get(0).getDocCount());
+                assertEquals(2, ranges.get(1).getDocCount());
+            },
+            field
+        );
+    }
+
+    public void testDoubleRangeKeys() throws IOException {
+        final String fieldName = "test";
+        MappedFieldType field = new NumberFieldMapper.NumberFieldType(fieldName, NumberType.DOUBLE);
+        testCase(
+            new RangeAggregationBuilder("0").field(fieldName).addRange(5, 6).addRange(6, 10.6).keyed(true),
+            new MatchAllDocsQuery(),
+            iw -> {
+                iw.addDocument(singleton(new SortedNumericDocValuesField(fieldName, NumericUtils.doubleToSortableLong(10))));
+                iw.addDocument(singleton(new SortedNumericDocValuesField(fieldName, NumericUtils.doubleToSortableLong(5.5D))));
+                iw.addDocument(singleton(new SortedNumericDocValuesField(fieldName, NumericUtils.doubleToSortableLong(6.7D))));
+            },
+            result -> {
+                InternalRange<?, ?> range = (InternalRange<?, ?>) result;
+                List<? extends InternalRange.Bucket> ranges = range.getBuckets();
+                assertEquals(2, ranges.size());
+                assertEquals("5.0-6.0", ranges.get(0).getKeyAsString());
+                assertEquals("6.0-10.6", ranges.get(1).getKeyAsString());
+                assertEquals("5.0", ranges.get(0).getFromAsString());
+                assertEquals("6.0", ranges.get(0).getToAsString());
+                assertEquals("6.0", ranges.get(1).getFromAsString());
+                assertEquals("10.6", ranges.get(1).getToAsString());
+                assertEquals(5.0D, ranges.get(0).getOriginalFrom(), 0.0000000000001);
+                assertEquals(6.0D, ranges.get(0).getOriginalTo(), 0.0000000000001);
+                assertEquals(6.0D, ranges.get(1).getOriginalFrom(), 0.0000000000001);
+                assertEquals(10.6D, ranges.get(1).getOriginalTo(), 0.0000000000001);
+                assertEquals(1, ranges.get(0).getDocCount());
+                assertEquals(2, ranges.get(1).getDocCount());
+            },
+            field
+        );
+    }
+
     /**
      * Confirm that a non-representable decimal stored as a float correctly follows the half-open interval rule
      */