Preserve half_float's precision in fields API

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

@@ -124,6 +124,18 @@ public enum NumberType {
         HALF_FLOAT("half_float", NumericType.HALF_FLOAT) {
             @Override
             public Float parse(Object value, boolean coerce) {
+                final float result = parseToFloat(value);
+                // Reduce the precision to what we actually index
+                return HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(result));
+            }
+
+            /**
+             * Parse a query parameter or {@code _source} value to a float,
+             * keeping float precision. Used by queries which need more
+             * precise control over their rounding behavior that
+             * {@link #parse(Object, boolean)} provides.
+             */
+            private float parseToFloat(Object value) {
                 final float result;
 
                 if (value instanceof Number) {
@@ -152,7 +164,7 @@ public Float parse(XContentParser parser, boolean coerce) throws IOException {
 
             @Override
             public Query termQuery(String field, Object value) {
-                float v = parse(value, false);
+                float v = parseToFloat(value);
                 return HalfFloatPoint.newExactQuery(field, v);
             }
 
@@ -161,7 +173,7 @@ public Query termsQuery(String field, Collection<?> values) {
                 float[] v = new float[values.size()];
                 int pos = 0;
                 for (Object value: values) {
-                    v[pos++] = parse(value, false);
+                    v[pos++] = parseToFloat(value);
                 }
                 return HalfFloatPoint.newSetQuery(field, v);
             }
@@ -173,14 +185,14 @@ public Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
                 float l = Float.NEGATIVE_INFINITY;
                 float u = Float.POSITIVE_INFINITY;
                 if (lowerTerm != null) {
-                    l = parse(lowerTerm, false);
+                    l = parseToFloat(lowerTerm);
                     if (includeLower) {
                         l = HalfFloatPoint.nextDown(l);
                     }
                     l = HalfFloatPoint.nextUp(l);
                 }
                 if (upperTerm != null) {
-                    u = parse(upperTerm, false);
+                    u = parseToFloat(upperTerm);
                     if (includeUpper) {
                         u = HalfFloatPoint.nextUp(u);
                     }

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

@@ -277,7 +277,7 @@ public void testConversions() {
         assertEquals("Value [2147483648] is out of range for an integer", e.getMessage());
         e = expectThrows(IllegalArgumentException.class, () -> NumberType.LONG.parse(10000000000000000000d, true));
         assertEquals("Value [1.0E19] is out of range for a long", e.getMessage());
-        assertEquals(1.1f, NumberType.HALF_FLOAT.parse(1.1, true));
+        assertEquals(1.0996094f, NumberType.HALF_FLOAT.parse(1.1, true));  // Half float loses a bit of precision even on 1.1....
         assertEquals(1.1f, NumberType.FLOAT.parse(1.1, true));
         assertEquals(1.1d, NumberType.DOUBLE.parse(1.1, true));
     }
@@ -648,4 +648,22 @@ public void testFetchSourceValue() throws IOException {
         assertEquals(List.of(2.71f), fetchSourceValue(nullValueMapper, ""));
         assertEquals(List.of(2.71f), fetchSourceValue(nullValueMapper, null));
     }
+
+    public void testFetchHalfFloatFromSource() throws IOException {
+        MappedFieldType mapper = new NumberFieldMapper.Builder("field", NumberType.HALF_FLOAT, false, true)
+            .build(new ContentPath())
+            .fieldType();
+        /*
+         * Half float loses a fair bit of precision compared to float but
+         * we still do floating point comparisons. The "funny" trailing
+         * {@code .000625} is, for example, the precision loss of using
+         * a half float reflected back into a float.
+         */
+        assertEquals(List.of(3.140625F), fetchSourceValue(mapper, 3.14));
+        assertEquals(List.of(3.140625F), fetchSourceValue(mapper, 3.14F));
+        assertEquals(List.of(3.0F), fetchSourceValue(mapper, 3));
+        assertEquals(List.of(42.90625F), fetchSourceValue(mapper, "42.9"));
+        assertEquals(List.of(47.125F), fetchSourceValue(mapper, 47.1231234));
+        assertEquals(List.of(3.140625F, 42.90625F), fetchSourceValues(mapper, 3.14, "foo", "42.9"));
+    }
 }

x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java

@@ -94,7 +94,7 @@ public void testAllTypesWithRequestToOldNodes() throws Exception {
             columns -> {
                 columns.add(columnInfo("geo_point_field", "geo_point"));
                 columns.add(columnInfo("float_field", "float"));
-                columns.add(columnInfo("half_float_field", "half_float"));
+                // Until #70653 is backported we won't assert that the half_float is returned with any kind of precision
             },
             (builder, fieldValues) -> {
                 Float randomFloat = randomFloat();
@@ -114,7 +114,8 @@ public void testAllTypesWithRequestToOldNodes() throws Exception {
                     fieldValues.put("geo_point_field", "POINT (-122.083843 37.386483)");
                     builder.append("\"float_field\":" + randomFloat + ",");
                     fieldValues.put("float_field", Double.valueOf(Float.valueOf(randomFloat).toString()));
-                    builder.append("\"half_float_field\":" + fieldValues.computeIfAbsent("half_float_field", v -> 123.456));
+                    builder.append("\"half_float_field\":\"123.456\"");
+                    // Until #70653 is backported we won't assert that the half_float is returned with any kind of precision
                 }
             }
         );
@@ -126,7 +127,7 @@ public void testAllTypesWithRequestToUpgradedNodes() throws Exception {
             columns -> {
                 columns.add(columnInfo("geo_point_field", "geo_point"));
                 columns.add(columnInfo("float_field", "float"));
-                columns.add(columnInfo("half_float_field", "half_float"));
+                // Until #70653 is backported we won't assert that the half_float is returned with any kind of precision
             },
             (builder, fieldValues) -> {
                 Float randomFloat = randomFloat();
@@ -143,7 +144,8 @@ public void testAllTypesWithRequestToUpgradedNodes() throws Exception {
                     fieldValues.put("geo_point_field", "POINT (-122.083843 37.386483)");
                     builder.append("\"float_field\":" + randomFloat + ",");
                     fieldValues.put("float_field", Double.valueOf(Float.valueOf(randomFloat).toString()));
-                    builder.append("\"half_float_field\":" + fieldValues.computeIfAbsent("half_float_field", v -> 123.456));
+                    builder.append("\"half_float_field\":\"123.456\"");
+                    // Until #70653 is backported we won't assert that the half_float is returned with any kind of precision
                 }
             }
         );

x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java

@@ -229,7 +229,9 @@ public void testCoerceForFloatingPointTypes() throws IOException {
 
         // because "coerce" is true, a "123.456" floating point number STRING should be converted to 123.456 as number
         // and converted to 123.5 for "scaled_float" type
-        expected.put("rows", singletonList(singletonList(isScaledFloat ? 123.5 : 123.456d)));
+        // and 123.4375 for "half_float" because that is all it stores.
+        double expectedNumber = isScaledFloat ? 123.5 : fieldType.equals("half_float") ? 123.4375 : 1234.56;
+        expected.put("rows", singletonList(singletonList(expectedNumber)));
         assertResponse(expected, runSql("SELECT " + fieldType + "_field FROM test"));
     }