Address feedback

Author: mayya-sharipova

docs/reference/mapping/types/numeric.asciidoc

@@ -116,7 +116,7 @@ The following parameters are accepted by numeric types:
 <<coerce,`coerce`>>::
 
     Try to convert strings to numbers and truncate fractions for integers.
-    Accepts `true` (default) and `false`. Not applicable for unsigned_long.
+    Accepts `true` (default) and `false`. Not applicable for `unsigned_long`.
 
 <<doc-values,`doc_values`>>::
 

docs/reference/mapping/types/unsigned_long.asciidoc

@@ -9,12 +9,7 @@
 
 Unsigned long is a numeric field type that represents an unsigned 64-bit
 integer with a minimum value of 0 and a maximum value of +2^64^-1+
-(from 0 to 18446744073709551615).
-
-At index-time, an indexed value is converted to the singed long range:
-[- 9223372036854775808, 9223372036854775807] by subtracting +2^63^+ from it
-and stored as a singed long taking 8 bytes.
-At query-time, the same conversion is done on query terms.
+(from 0 to 18446744073709551615 inclusive).
 
 [source,console]
 --------------------------------------------------
@@ -63,7 +58,11 @@ GET /my_index/_search
 --------------------------------
 //TEST[continued]
 
-Range queries can contain ranges with decimal parts.
+Range query terms can contain values with decimal parts.
+In this case {es} converts them to integer values:
+`gte` and `gt` terms are converted to the nearest integer up inclusive,
+and `lt` and `lte` ranges are converted to the nearest integer down inclusive.
+
 It is recommended to pass ranges as strings to ensure they are parsed
 without any loss of precision.
 
@@ -83,13 +82,13 @@ GET /my_index/_search
 --------------------------------
 //TEST[continued]
 
-WARNING: Unlike term and range queries, sorting and aggregations on
-unsigned_long data may return imprecise results. For sorting and aggregations
-double representation of unsigned longs is used, which means that long values
-are first converted to double values. During this conversion,
-for long values greater than +2^53^+ there could be some loss of
-precision for the least significant digits. Long values less than +2^53^+
-are converted accurately.
+
+For queries with sort on an `unsigned_long` field,
+for a particular document {es} returns a sort value of the type `Long`
+if the value of this document is within the range of long values,
+or of the type `BigIntger` if the value exceeds this range.
+
+WARNING: Not all {es} clients can properly handle big integer values.
 
 [source,console]
 --------------------------------
@@ -98,15 +97,25 @@ GET /my_index/_search
     "query": {
         "match_all" : {}
     },
-    "sort" : {"my_counter" : "desc"}  <1>
+    "sort" : {"my_counter" : "desc"}
 }
 --------------------------------
 //TEST[continued]
-<1> As both document values: "18446744073709551614" and "18446744073709551615"
-are converted to the same double value: "1.8446744073709552E19", this
-descending sort may return imprecise results, as the document with a lower
-value of "18446744073709551614" may come before the document
-with a higher value of "18446744073709551615".
+
+Similarly to sort values, script values of an `unsigned_long` field
+produce `BigInteger` or `Long` values. The same values: `BigInteger` or
+`Long` are returned as keys for `terms` aggregation.
+
+==== Queries with mixed numeric types
+
+Search queries across several numeric types one of which `unsigned_long` are supported,
+except queries with sort. Thus, a sort query across two indexes where the same field
+is `unsigned_long` in one index, and `long` in another, doesn't produce correct results
+and must be avoided. If there is a need for a such kind of sorting, script based
+sorting can be used instead.
+Aggregations across several numeric types one of which `unsigned_long` are supported,
+except a terms aggregation.
+
 
 [[unsigned-long-params]]
 ==== Parameters for unsigned long fields

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java

@@ -206,8 +206,6 @@ <T> Map<String, T> map(
 
     long longValue() throws IOException;
 
-    long unsignedLongValue() throws IOException;
-
     float floatValue() throws IOException;
 
     double doubleValue() throws IOException;

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java

@@ -224,11 +224,6 @@ public long longValue() throws IOException {
         return parser.longValue();
     }
 
-    @Override
-    public long unsignedLongValue() throws IOException {
-        return parser.unsignedLongValue();
-    }
-
     @Override
     public float floatValue() throws IOException {
         return parser.floatValue();

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java

@@ -30,7 +30,6 @@
 import org.elasticsearch.core.internal.io.IOUtils;
 
 import java.io.IOException;
-import java.math.BigInteger;
 import java.nio.CharBuffer;
 
 public class JsonXContentParser extends AbstractXContentParser {
@@ -167,26 +166,6 @@ public long doLongValue() throws IOException {
         return parser.getLongValue();
     }
 
-    @Override
-    protected long doUnsignedLongValue() throws IOException {
-        JsonParser.NumberType numberType = parser.getNumberType();
-        if ((numberType == JsonParser.NumberType.INT) || (numberType == JsonParser.NumberType.LONG)) {
-            long longValue = parser.getLongValue();
-            if (longValue < 0) {
-                throw new IllegalArgumentException("Value [" + longValue + "] is out of range for unsigned long.");
-            }
-            return longValue;
-        } else if (numberType == JsonParser.NumberType.BIG_INTEGER) {
-            BigInteger bigIntegerValue = parser.getBigIntegerValue();
-            if (bigIntegerValue.compareTo(BIGINTEGER_MAX_UNSIGNED_LONG_VALUE) > 0 || bigIntegerValue.compareTo(BigInteger.ZERO) < 0) {
-                throw new IllegalArgumentException("Value [" + bigIntegerValue + "] is out of range for unsigned long");
-            }
-            return bigIntegerValue.longValue();
-        } else { // for all other value types including numbers with decimal parts
-            throw new IllegalArgumentException("For input string: [" + parser.getValueAsString() + "].");
-        }
-    }
-
     @Override
     public float doFloatValue() throws IOException {
         return parser.getFloatValue();

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java

@@ -46,8 +46,6 @@ public abstract class AbstractXContentParser implements XContentParser {
     // references to this policy decision throughout the codebase and find
     // and change any code that needs to apply an alternative policy.
     public static final boolean DEFAULT_NUMBER_COERCE_POLICY = true;
-    public static BigInteger BIGINTEGER_MAX_UNSIGNED_LONG_VALUE = BigInteger.ONE.shiftLeft(64).subtract(BigInteger.ONE); // 2^64 -1
-
 
     private static void checkCoerceString(boolean coerce, Class<? extends Number> clazz) {
         if (!coerce) {
@@ -210,26 +208,8 @@ public long longValue(boolean coerce) throws IOException {
         return result;
     }
 
-    @Override
-    public long unsignedLongValue() throws IOException {
-        Token token = currentToken();
-        if (token == Token.VALUE_STRING) {
-            return Long.parseUnsignedLong(text());
-        }
-        long result = doUnsignedLongValue();
-        return result;
-    }
-
     protected abstract long doLongValue() throws IOException;
 
-    /**
-     * Returns an unsigned long value of the current numeric token.
-     * The method must check for proper boundaries: [0; 2^64-1], and also check that it doesn't have a decimal part.
-     * An exception is raised if any of the conditions is violated.
-     * Numeric tokens greater than Long.MAX_VALUE must be returned as negative values.
-     */
-    protected abstract long doUnsignedLongValue() throws IOException;
-
     @Override
     public float floatValue() throws IOException {
         return floatValue(DEFAULT_NUMBER_COERCE_POLICY);

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java

@@ -73,26 +73,6 @@ protected long doLongValue() throws IOException {
         return numberValue().longValue();
     }
 
-    @Override
-    protected long doUnsignedLongValue() throws IOException {
-        Number value = numberValue();
-        if ((value instanceof Integer) || (value instanceof Long) || (value instanceof Short) || (value instanceof Byte)) {
-            long longValue = value.longValue();
-            if (longValue < 0) {
-                throw new IllegalArgumentException("Value [" + longValue + "] is out of range for unsigned long.");
-            }
-            return longValue;
-        } else if (value instanceof BigInteger) {
-            BigInteger bigIntegerValue = (BigInteger) value;
-            if (bigIntegerValue.compareTo(BIGINTEGER_MAX_UNSIGNED_LONG_VALUE) > 0 || bigIntegerValue.compareTo(BigInteger.ZERO) < 0) {
-                throw new IllegalArgumentException("Value [" + bigIntegerValue + "] is out of range for unsigned long.");
-            }
-            return bigIntegerValue.longValue();
-        } else {
-            throw new IllegalArgumentException("For input string: [" + value.toString() + "].");
-        }
-    }
-
     @Override
     protected float doFloatValue() throws IOException {
         return numberValue().floatValue();

modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java

@@ -29,6 +29,7 @@
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
+import java.math.BigInteger;
 import java.time.ZonedDateTime;
 import java.util.BitSet;
 import java.util.Collections;
@@ -734,6 +735,8 @@ public static double defTodoubleImplicit(final Object value) {
             return (float)value;
         } else if (value instanceof Double) {
             return (double)value;
+        } else if (value instanceof BigInteger) {
+            return ((BigInteger)value).doubleValue();
         } else {
             throw new ClassCastException("cannot implicitly cast " +
                     "def [" + PainlessLookupUtility.typeToUnboxedType(value.getClass()).getCanonicalName() + "] to " +
@@ -866,7 +869,8 @@ public static double defTodoubleExplicit(final Object value) {
                 value instanceof Integer ||
                 value instanceof Long    ||
                 value instanceof Float   ||
-                value instanceof Double
+                value instanceof Double  ||
+                value instanceof BigInteger
         ) {
             return ((Number)value).doubleValue();
         } else {
@@ -1004,7 +1008,9 @@ public static Double defToDoubleImplicit(final Object value) {
         } else if (value instanceof Float) {
             return (double)(float)value;
         } else if (value instanceof Double) {
-            return (Double)value;
+            return (Double) value;
+        } else if (value instanceof BigInteger) {
+            return ((BigInteger)value).doubleValue();
         } else {
             throw new ClassCastException("cannot implicitly cast " +
                     "def [" + PainlessLookupUtility.typeToUnboxedType(value.getClass()).getCanonicalName() + "] to " +
@@ -1151,7 +1157,8 @@ public static Double defToDoubleExplicit(final Object value) {
                 value instanceof Integer ||
                 value instanceof Long    ||
                 value instanceof Float   ||
-                value instanceof Double
+                value instanceof Double  ||
+                value instanceof BigInteger
         ) {
             return ((Number)value).doubleValue();
         } else {

modules/lang-painless/src/test/java/org/elasticsearch/painless/DefCastTests.java

@@ -166,6 +166,7 @@ public void testdefTodoubleImplicit() {
         assertEquals((double)0, exec("def d = Long.valueOf(0); double b = d; b"));
         assertEquals((double)0, exec("def d = Float.valueOf(0); double b = d; b"));
         assertEquals((double)0, exec("def d = Double.valueOf(0); double b = d; b"));
+        assertEquals((double)0, exec("def d = BigInteger.valueOf(0); double b = d; b"));
         expectScriptThrows(ClassCastException.class, () -> exec("def d = new ArrayList(); double b = d;"));
     }
 
@@ -485,7 +486,7 @@ public void testdefToFloatImplicit() {
         expectScriptThrows(ClassCastException.class, () -> exec("def d = Double.valueOf(0); Float b = d;"));
         expectScriptThrows(ClassCastException.class, () -> exec("def d = new ArrayList(); Float b = d;"));
     }
-    
+
     public void testdefToDoubleImplicit() {
         expectScriptThrows(ClassCastException.class, () -> exec("def d = 'string'; Double b = d;"));
         expectScriptThrows(ClassCastException.class, () -> exec("def d = true; Double b = d;"));

server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java

@@ -51,6 +51,7 @@
 import java.io.FilterInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.math.BigInteger;
 import java.nio.file.AccessDeniedException;
 import java.nio.file.AtomicMoveNotSupportedException;
 import java.nio.file.DirectoryNotEmptyException;
@@ -348,6 +349,11 @@ public Long readOptionalLong() throws IOException {
         return null;
     }
 
+    public BigInteger readBigInteger() throws IOException {
+        return new BigInteger(readString());
+    }
+
+
     @Nullable
     public Text readOptionalText() throws IOException {
         int length = readInt();
@@ -760,6 +766,8 @@ public Object readGenericValue() throws IOException {
                 return readCollection(StreamInput::readGenericValue, LinkedHashSet::new, Collections.emptySet());
             case 25:
                 return readCollection(StreamInput::readGenericValue, HashSet::new, Collections.emptySet());
+            case 26:
+                return readBigInteger();
             default:
                 throw new IOException("Can't read unknown type [" + type + "]");
         }