[7.17] refactor: use the original from/to values when creating the bu…

…cket (#83262) (#83339) The range bucket values should always be set to the original `from` and `to` values. These values are the ones used by the serialisation logic and are the values the client expects in the response. Also, the key for a Range is made optional, avoiding unnecessary serialisation and deserialisation and the actual key generation happens only if required (calling 'getKey' or 'getKeyAsString' directly or at key serialisation time). A Range does not actually need a key to accomplish its purpose ('from' and 'to' are the only required parameters), other than propagating a user-specified key for the bucket collecting documents for that range. (cherry picked from commit a03cd92)
elastic · Jan 31, 2022 · 7f5623f · 7f5623f
1 parent 8f5f8cf
commit 7f5623f
Show file tree

Hide file tree

Showing 12 changed files with 107 additions and 198 deletions.
diff --git a/.../java/org/elasticsearch/search/aggregations/bucket/range/DateRangeAggregationBuilder.java b/.../java/org/elasticsearch/search/aggregations/bucket/range/DateRangeAggregationBuilder.java
@@ -364,7 +364,7 @@ protected DateRangeAggregatorFactory innerBuild(
             } else if (Double.isFinite(to)) {
                 to = parser.parseDouble(Long.toString((long) to), false, context::nowInMillis);
             }
-            return new RangeAggregator.Range(range.getKey(), from, from, fromAsString, to, to, toAsString);
+            return new RangeAggregator.Range(range.getKey(), from, fromAsString, to, toAsString);
         });
         if (ranges.length == 0) {
             throw new IllegalArgumentException("No [ranges] specified for the [" + this.getName() + "] aggregation");

diff --git a/...src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalBinaryRange.java b/...src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalBinaryRange.java
@@ -54,7 +54,7 @@ public Bucket(
         ) {
             this.format = format;
             this.keyed = keyed;
-            this.key = key != null ? key : generateKey(from, to, format);
+            this.key = key;
             this.from = from;
             this.to = to;
             this.docCount = docCount;
@@ -69,8 +69,7 @@ private static String generateKey(BytesRef from, BytesRef to, DocValueFormat for
         }
 
         private static Bucket createFromStream(StreamInput in, DocValueFormat format, boolean keyed) throws IOException {
-            String key = in.getVersion().onOrAfter(Version.V_6_4_0) ? in.readString() : in.readOptionalString();
-
+            String key = in.getVersion().onOrAfter(Version.V_7_17_0) ? in.readOptionalString() : in.readString();
             BytesRef from = in.readBoolean() ? in.readBytesRef() : null;
             BytesRef to = in.readBoolean() ? in.readBytesRef() : null;
             long docCount = in.readLong();
@@ -81,10 +80,10 @@ private static Bucket createFromStream(StreamInput in, DocValueFormat format, bo
 
         @Override
         public void writeTo(StreamOutput out) throws IOException {
-            if (out.getVersion().onOrAfter(Version.V_6_4_0)) {
-                out.writeString(key);
-            } else {
+            if (out.getVersion().onOrAfter(Version.V_7_17_0)) {
                 out.writeOptionalString(key);
+            } else {
+                out.writeString(key == null ? generateKey(from, to, format) : key);
             }
             out.writeBoolean(from != null);
             if (from != null) {
@@ -100,12 +99,12 @@ public void writeTo(StreamOutput out) throws IOException {
 
         @Override
         public Object getKey() {
-            return key;
+            return getKeyAsString();
         }
 
         @Override
         public String getKeyAsString() {
-            return key;
+            return this.key == null ? generateKey(this.from, this.to, this.format) : this.key;
         }
 
         @Override
@@ -120,7 +119,7 @@ public Aggregations getAggregations() {
 
         @Override
         public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-            String key = this.key;
+            final String key = getKeyAsString();
             if (keyed) {
                 builder.startObject(key);
             } else {

diff --git a/...r/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalDateRange.java b/...r/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalDateRange.java
@@ -28,29 +28,25 @@ public static class Bucket extends InternalRange.Bucket {
         public Bucket(
             String key,
             double from,
-            double originalFrom,
             double to,
-            double originalTo,
             long docCount,
             List<InternalAggregation> aggregations,
             boolean keyed,
             DocValueFormat formatter
         ) {
-            super(key, from, originalFrom, to, originalTo, docCount, InternalAggregations.from(aggregations), keyed, formatter);
+            super(key, from, to, 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, originalFrom, to, originalTo, docCount, aggregations, keyed, formatter);
+            super(key, from, to, docCount, aggregations, keyed, formatter);
         }
 
         @Override
@@ -75,14 +71,6 @@ private Double internalGetTo() {
             return to;
         }
 
-        private Double internalGetOriginalFrom() {
-            return originalFrom;
-        }
-
-        private Double internalGetOriginalTo() {
-            return originalTo;
-        }
-
         @Override
         protected InternalRange.Factory<Bucket, ?> getFactory() {
             return FACTORY;
@@ -124,25 +112,21 @@ public InternalDateRange create(List<Bucket> ranges, InternalDateRange prototype
         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, originalFrom, to, originalTo, docCount, aggregations, keyed, formatter);
+            return new Bucket(key, from, to, docCount, aggregations, keyed, formatter);
         }
 
         @Override
         public Bucket createBucket(InternalAggregations aggregations, Bucket prototype) {
             return new Bucket(
                 prototype.getKey(),
                 prototype.internalGetFrom(),
-                prototype.internalGetOriginalFrom(),
                 prototype.internalGetTo(),
-                prototype.internalGetOriginalTo(),
                 prototype.getDocCount(),
                 aggregations,
                 prototype.getKeyed(),

diff --git a/...src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalGeoDistance.java b/...src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalGeoDistance.java
@@ -23,17 +23,8 @@ public class InternalGeoDistance extends InternalRange<InternalGeoDistance.Bucke
 
     static class Bucket extends InternalRange.Bucket {
 
-        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);
+        Bucket(String key, double from, double to, long docCount, InternalAggregations aggregations, boolean keyed) {
+            super(key, from, to, docCount, aggregations, keyed, DocValueFormat.RAW);
         }
 
         @Override
@@ -77,25 +68,21 @@ public InternalGeoDistance create(List<Bucket> ranges, InternalGeoDistance proto
         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, originalFrom, to, originalTo, docCount, aggregations, keyed);
+            return new Bucket(key, from, to, docCount, aggregations, keyed);
         }
 
         @Override
         public Bucket createBucket(InternalAggregations aggregations, Bucket prototype) {
             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()