fix: reduce float and half-float values to their stored precision (#83213)

The bug is a result of reducing precision of float values. If the filed the range aggregation is applied to is a long field, we try to reduce the precision of the rage from and/or to values to long. This results in an exception at parsing time happening when parsing a long with decimal positions.

Here we parse FLOAT and HALF-FLOAT values actually reducing their precision. For other types, including long we do nothing, or better, we do not do any "precision-reduction" action in method reduceToStoredPrecision.

docs/changelog/83213.yaml

@@ -0,0 +1,5 @@
+pr: 83213
+summary: "Fix: reduce float and half-float values to their stored precision"
+area: Aggregations
+type: bug
+issues: []

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

@@ -28,6 +28,17 @@ setup:
                 type: date
                 format: strict_date_time||strict_date
 
+  - do:
+      indices.create:
+        index: long_value_test
+        body:
+          settings:
+            number_of_replicas: 0
+          mappings:
+            properties:
+              long:
+                type: long
+
   - do:
       cluster.health:
         wait_for_status: yellow
@@ -37,15 +48,22 @@ setup:
         index: test
         refresh: true
         body:
-         - {"index": {}}
-         - { "double" : 42.1, "long": 25, "float": 0.01, "half_float": 0.01 }
-         - {"index": {}}
-         - { "double" : 100.7, "long": 80, "float": 0.03, "half_float": 0.0152 }
-         - {"index": {}}
-         - { "double" : 50.5, "long":  75, "float": 0.04, "half_float": 0.04 }
-# For testing missing values
-         - {"index": {}}
-         - {}
+          - {"index": {}}
+          - { "double" : 42.1, "long": 25, "float": 0.01, "half_float": 0.01 }
+          - {"index": {}}
+          - { "double" : 100.7, "long": 80, "float": 0.03, "half_float": 0.0152 }
+          - {"index": {}}
+          - { "double" : 50.5, "long":  75, "float": 0.04, "half_float": 0.04 }
+          # For testing missing values
+          - {"index": {}}
+          - {}
+  - do:
+      bulk:
+        index: long_value_test
+        refresh: true
+        body:
+          - { "index": { } }
+          - { "long": -9223372036854775808 }
 
   - do:
       bulk:
@@ -199,6 +217,46 @@ setup:
   - is_false:  aggregations.float_range.buckets.2.to
   - match: { aggregations.float_range.buckets.2.doc_count: 3 }
 
+---
+"Double range on long field":
+  - skip:
+      version: " - 8.0.99"
+      reason: Bug fixed in 8.1.0
+  - do:
+      search:
+        index: test
+        body:
+          size: 0
+          aggs:
+            double_range:
+              range:
+                field: "long"
+                ranges:
+                  -
+                    to: 24.9
+                  -
+                    from: 24.9
+                    to: 79.9
+                  -
+                    from: 79.9
+
+  - match: { hits.total.relation: "eq" }
+  - match: { hits.total.value: 4 }
+  - length: { aggregations.double_range.buckets: 3 }
+  - match: { aggregations.double_range.buckets.0.key: "*-24.9" }
+  - is_false: aggregations.double_range.buckets.0.from
+  - match: { aggregations.double_range.buckets.0.to: 24.9 }
+  - match: { aggregations.double_range.buckets.0.doc_count: 0 }
+  - match: { aggregations.double_range.buckets.1.key: "24.9-79.9" }
+  - match: { aggregations.double_range.buckets.1.from: 24.9 }
+  - match: { aggregations.double_range.buckets.1.to: 79.9 }
+  - match: { aggregations.double_range.buckets.1.doc_count: 2 }
+  - match: { aggregations.double_range.buckets.2.key: "79.9-*" }
+  - match: { aggregations.double_range.buckets.2.from: 79.9 }
+  - is_false:  aggregations.double_range.buckets.2.to
+  - match: { aggregations.double_range.buckets.2.doc_count: 1 }
+
+
 ---
 "Double range no decimal":
   - do:
@@ -409,3 +467,27 @@ setup:
   - match: { aggregations.date_range.buckets.0.from_as_string: "2021-05-01T00:00:00.000Z" }
   - match: { aggregations.date_range.buckets.0.to: 1620172800000 }
   - match: { aggregations.date_range.buckets.0.to_as_string: "2021-05-05T00:00:00.000Z" }
+
+---
+"Min and max long range bounds":
+  - skip:
+      version: " - 8.0.99"
+      reason: Bug fixed in 8.1.0
+  - do:
+      search:
+        index: long_value_test
+        body:
+          size: 0
+          aggs:
+            long_range:
+              range:
+                field: "long"
+                ranges:
+                  { from: -9223372036854775808, to: 9223372036854775807 }
+  - match: { hits.total.relation: "eq" }
+  - match: { hits.total.value: 1 }
+  - length: { aggregations.long_range.buckets: 1 }
+  - match: { aggregations.long_range.buckets.0.key: "-9.223372036854776E18-9.223372036854776E18" }
+  - match: { aggregations.long_range.buckets.0.from: -9.223372036854776E18 }
+  - match: { aggregations.long_range.buckets.0.to: 9.223372036854776E18 }
+  - match: { aggregations.long_range.buckets.0.doc_count: 1 }

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

@@ -235,6 +235,11 @@ public class NumberFieldMapper extends FieldMapper {
                 return HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(result));
             }
 
+            @Override
+            public double reduceToStoredPrecision(double value) {
+                return parse(value, false).doubleValue();
+            }
+
             /**
              * Parse a query parameter or {@code _source} value to a float,
              * keeping float precision. Used by queries which need more
@@ -379,6 +384,11 @@ public class NumberFieldMapper extends FieldMapper {
                 return result;
             }
 
+            @Override
+            public double reduceToStoredPrecision(double value) {
+                return parse(value, false).doubleValue();
+            }
+
             @Override
             public Number parsePoint(byte[] value) {
                 return FloatPoint.decodeDimension(value, 0);
@@ -1167,6 +1177,18 @@ public class NumberFieldMapper extends FieldMapper {
         }
 
         public abstract IndexFieldData.Builder getFieldDataBuilder(String name);
+
+        /**
+         * Adjusts a value to the value it would have been had it been parsed by that mapper
+         * and then cast up to a double. This is meant to be an entry point to manipulate values
+         * before the actual value is parsed.
+         *
+         * @param value the value to reduce to the field stored value
+         * @return the double value
+         */
+        public double reduceToStoredPrecision(double value) {
+            return ((Number) value).doubleValue();
+        }
     }
 
     public static class NumberFieldType extends SimpleMappedFieldType {
@@ -1241,7 +1263,7 @@ public class NumberFieldMapper extends FieldMapper {
                 // Trying to parse infinite values into ints/longs throws. Understandably.
                 return value;
             }
-            return type.parse(value, false).doubleValue();
+            return type.reduceToStoredPrecision(value);
         }
 
         public NumericType numericType() {

server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorTests.java

@@ -121,6 +121,26 @@ public class RangeAggregatorTests extends AggregatorTestCase {
         );
     }
 
+    public void testMinAndMaxLongRangeBounds() throws IOException {
+        final String fieldName = "long_field";
+        MappedFieldType field = new NumberFieldMapper.NumberFieldType(fieldName, NumberType.LONG);
+        double from = Long.valueOf(Long.MIN_VALUE).doubleValue();
+        double to = Long.valueOf(Long.MAX_VALUE).doubleValue();
+        testCase(
+            new RangeAggregationBuilder("0").field(fieldName).addRange(Long.MIN_VALUE, Long.MAX_VALUE),
+            new MatchAllDocsQuery(),
+            iw -> { iw.addDocument(singleton(new NumericDocValuesField(fieldName, randomLong()))); },
+            result -> {
+                InternalRange<?, ?> range = (InternalRange<?, ?>) result;
+                List<? extends InternalRange.Bucket> ranges = range.getBuckets();
+                assertEquals(1, ranges.size());
+                assertEquals(from + "-" + to, ranges.get(0).getKeyAsString());
+                assertEquals(1, ranges.get(0).getDocCount());
+            },
+            field
+        );
+    }
+
     public void testFloatRangeFromAndToValues() throws IOException {
         final String fieldName = "test";
         MappedFieldType field = new NumberFieldMapper.NumberFieldType(fieldName, NumberType.FLOAT);
@@ -185,6 +205,52 @@ public class RangeAggregatorTests extends AggregatorTestCase {
         );
     }
 
+    public void testDoubleRangeWithLongField() throws IOException {
+        final String fieldName = "long_field";
+        MappedFieldType field = new NumberFieldMapper.NumberFieldType(fieldName, NumberType.LONG);
+        testCase(
+            new RangeAggregationBuilder("0").field(fieldName).addRange(990.0, 999.9).addUnboundedFrom(999.9),
+            new MatchAllDocsQuery(),
+            iw -> {
+                iw.addDocument(singleton(new NumericDocValuesField(fieldName, 998)));
+                iw.addDocument(singleton(new NumericDocValuesField(fieldName, 999)));
+                iw.addDocument(singleton(new NumericDocValuesField(fieldName, 1000)));
+                iw.addDocument(singleton(new NumericDocValuesField(fieldName, 1001)));
+            },
+            result -> {
+                InternalRange<?, ?> range = (InternalRange<?, ?>) result;
+                List<? extends InternalRange.Bucket> ranges = range.getBuckets();
+                assertEquals(2, ranges.size());
+                assertEquals(2, ranges.get(0).getDocCount());
+                assertEquals(2, ranges.get(1).getDocCount());
+            },
+            field
+        );
+    }
+
+    public void testDoubleRangeWithIntegerField() throws IOException {
+        final String fieldName = "integer_field";
+        MappedFieldType field = new NumberFieldMapper.NumberFieldType(fieldName, NumberType.INTEGER);
+        testCase(
+            new RangeAggregationBuilder("0").field(fieldName).addRange(990.0, 999.9).addUnboundedFrom(999.9),
+            new MatchAllDocsQuery(),
+            iw -> {
+                iw.addDocument(singleton(new NumericDocValuesField(fieldName, 998)));
+                iw.addDocument(singleton(new NumericDocValuesField(fieldName, 999)));
+                iw.addDocument(singleton(new NumericDocValuesField(fieldName, 1000)));
+                iw.addDocument(singleton(new NumericDocValuesField(fieldName, 1001)));
+            },
+            result -> {
+                InternalRange<?, ?> range = (InternalRange<?, ?>) result;
+                List<? extends InternalRange.Bucket> ranges = range.getBuckets();
+                assertEquals(2, ranges.size());
+                assertEquals(2, 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
      */