From 860a8a7b235a91d6927ac8e2171f039c23b8de3e Mon Sep 17 00:00:00 2001
From: Armin Braun <me@obrown.io>
Date: Fri, 11 Jan 2019 08:07:55 +0100
Subject: [PATCH] Improve Precision for scaled_float (#37169)

* Use `toString` and `Bigdecimal` parsing to get intuitive behaviour for `scaled_float` as discussed in #32570
* Closes #32570
---
 .../index/mapper/ScaledFloatFieldMapper.java  | 24 ++++++++++++++-----
 .../mapper/ScaledFloatFieldTypeTests.java     |  2 ++
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java
index 07ee5b5dc62..d3719ec884f 100644
--- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java
+++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java
@@ -62,6 +62,7 @@ import org.elasticsearch.search.MultiValueMode;
 import org.joda.time.DateTimeZone;
 
 import java.io.IOException;
+import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
@@ -227,8 +228,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
         @Override
         public Query termQuery(Object value, QueryShardContext context) {
             failIfNotIndexed();
-            double queryValue = parse(value);
-            long scaledValue = Math.round(queryValue * scalingFactor);
+            long scaledValue = Math.round(scale(value));
             Query query = NumberFieldMapper.NumberType.LONG.termQuery(name(), scaledValue);
             if (boost() != 1f) {
                 query = new BoostQuery(query, boost());
@@ -241,8 +241,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
             failIfNotIndexed();
             List<Long> scaledValues = new ArrayList<>(values.size());
             for (Object value : values) {
-                double queryValue = parse(value);
-                long scaledValue = Math.round(queryValue * scalingFactor);
+                long scaledValue = Math.round(scale(value));
                 scaledValues.add(scaledValue);
             }
             Query query = NumberFieldMapper.NumberType.LONG.termsQuery(name(), Collections.unmodifiableList(scaledValues));
@@ -257,7 +256,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
             failIfNotIndexed();
             Long lo = null;
             if (lowerTerm != null) {
-                double dValue = parse(lowerTerm) * scalingFactor;
+                double dValue = scale(lowerTerm);
                 if (includeLower == false) {
                     dValue = Math.nextUp(dValue);
                 }
@@ -265,7 +264,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
             }
             Long hi = null;
             if (upperTerm != null) {
-                double dValue = parse(upperTerm) * scalingFactor;
+                double dValue = scale(upperTerm);
                 if (includeUpper == false) {
                     dValue = Math.nextDown(dValue);
                 }
@@ -326,6 +325,19 @@ public class ScaledFloatFieldMapper extends FieldMapper {
         public int hashCode() {
             return 31 * super.hashCode() + Double.hashCode(scalingFactor);
         }
+
+        /**
+         * Parses input value and multiplies it with the scaling factor.
+         * Uses the round-trip of creating a {@link BigDecimal} from the stringified {@code double}
+         * input to ensure intuitively exact floating point operations.
+         * (e.g. for a scaling factor of 100, JVM behaviour results in {@code 79.99D * 100 ==> 7998.99..} compared to
+         * {@code scale(79.99) ==> 7999})
+         * @param input Input value to parse floating point num from
+         * @return Scaled value
+         */
+        private double scale(Object input) {
+            return new BigDecimal(Double.toString(parse(input))).multiply(BigDecimal.valueOf(scalingFactor)).doubleValue();
+        }
     }
 
     private Explicit<Boolean> ignoreMalformed;
diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java
index 1d88022b3e0..4389e809bfb 100644
--- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java
+++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java
@@ -140,6 +140,8 @@ public class ScaledFloatFieldTypeTests extends FieldTypeTestCase {
         assertEquals("scaled_float:[-9223372036854775808 TO 10]", scaledFloatQ.toString());
         scaledFloatQ = ft.rangeQuery(null, 0.105, true, true, null);
         assertEquals("scaled_float:[-9223372036854775808 TO 10]", scaledFloatQ.toString());
+        scaledFloatQ = ft.rangeQuery(null, 79.99, true, true, null);
+        assertEquals("scaled_float:[-9223372036854775808 TO 7999]", scaledFloatQ.toString());
     }
 
     public void testRoundsLowerBoundCorrectly() {