diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java index 95fe08d96c3..be128b2f212 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java @@ -133,7 +133,7 @@ public abstract class AbstractXContentParser implements XContentParser { Token token = currentToken(); if (token == Token.VALUE_STRING) { checkCoerceString(coerce, Short.class); - return Short.parseShort(text()); + return (short) Double.parseDouble(text()); } short result = doShortValue(); ensureNumberConversion(coerce, result, Short.class); @@ -147,13 +147,12 @@ public abstract class AbstractXContentParser implements XContentParser { return intValue(DEFAULT_NUMBER_COERCE_POLICY); } - @Override public int intValue(boolean coerce) throws IOException { Token token = currentToken(); if (token == Token.VALUE_STRING) { checkCoerceString(coerce, Integer.class); - return Integer.parseInt(text()); + return (int) Double.parseDouble(text()); } int result = doIntValue(); ensureNumberConversion(coerce, result, Integer.class); @@ -172,7 +171,13 @@ public abstract class AbstractXContentParser implements XContentParser { Token token = currentToken(); if (token == Token.VALUE_STRING) { checkCoerceString(coerce, Long.class); - return Long.parseLong(text()); + // longs need special handling so we don't lose precision while parsing + String stringValue = text(); + try { + return Long.parseLong(stringValue); + } catch (NumberFormatException e) { + return (long) Double.parseDouble(stringValue); + } } long result = doLongValue(); ensureNumberConversion(coerce, result, Long.class); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 62aab947f95..81108b02be8 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -312,13 +312,7 @@ public class NumberFieldMapper extends FieldMapper { DOUBLE("double", NumericType.DOUBLE) { @Override Double parse(Object value, boolean coerce) { - if (value instanceof Number) { - return ((Number) value).doubleValue(); - } - if (value instanceof BytesRef) { - value = ((BytesRef) value).utf8ToString(); - } - return Double.parseDouble(value.toString()); + return objectToDouble(value); } @Override @@ -389,20 +383,20 @@ public class NumberFieldMapper extends FieldMapper { BYTE("byte", NumericType.BYTE) { @Override Byte parse(Object value, boolean coerce) { + double doubleValue = objectToDouble(value); + + if (doubleValue < Byte.MIN_VALUE || doubleValue > Byte.MAX_VALUE) { + throw new IllegalArgumentException("Value [" + value + "] is out of range for a byte"); + } + if (!coerce && doubleValue % 1 != 0) { + throw new IllegalArgumentException("Value [" + value + "] has a decimal part"); + } + if (value instanceof Number) { - double doubleValue = ((Number) value).doubleValue(); - if (doubleValue < Byte.MIN_VALUE || doubleValue > Byte.MAX_VALUE) { - throw new IllegalArgumentException("Value [" + value + "] is out of range for a byte"); - } - if (!coerce && doubleValue % 1 != 0) { - throw new IllegalArgumentException("Value [" + value + "] has a decimal part"); - } return ((Number) value).byteValue(); } - if (value instanceof BytesRef) { - value = ((BytesRef) value).utf8ToString(); - } - return Byte.parseByte(value.toString()); + + return (byte) doubleValue; } @Override @@ -445,29 +439,25 @@ public class NumberFieldMapper extends FieldMapper { SHORT("short", NumericType.SHORT) { @Override Short parse(Object value, boolean coerce) { + double doubleValue = objectToDouble(value); + + if (doubleValue < Short.MIN_VALUE || doubleValue > Short.MAX_VALUE) { + throw new IllegalArgumentException("Value [" + value + "] is out of range for a short"); + } + if (!coerce && doubleValue % 1 != 0) { + throw new IllegalArgumentException("Value [" + value + "] has a decimal part"); + } + if (value instanceof Number) { - double doubleValue = ((Number) value).doubleValue(); - if (doubleValue < Short.MIN_VALUE || doubleValue > Short.MAX_VALUE) { - throw new IllegalArgumentException("Value [" + value + "] is out of range for a short"); - } - if (!coerce && doubleValue % 1 != 0) { - throw new IllegalArgumentException("Value [" + value + "] has a decimal part"); - } return ((Number) value).shortValue(); } - if (value instanceof BytesRef) { - value = ((BytesRef) value).utf8ToString(); - } - return Short.parseShort(value.toString()); + + return (short) doubleValue; } @Override Short parse(XContentParser parser, boolean coerce) throws IOException { - int value = parser.intValue(coerce); - if (value < Short.MIN_VALUE || value > Short.MAX_VALUE) { - throw new IllegalArgumentException("Value [" + value + "] is out of range for a short"); - } - return (short) value; + return parser.shortValue(coerce); } @Override @@ -501,20 +491,20 @@ public class NumberFieldMapper extends FieldMapper { INTEGER("integer", NumericType.INT) { @Override Integer parse(Object value, boolean coerce) { + double doubleValue = objectToDouble(value); + + if (doubleValue < Integer.MIN_VALUE || doubleValue > Integer.MAX_VALUE) { + throw new IllegalArgumentException("Value [" + value + "] is out of range for an integer"); + } + if (!coerce && doubleValue % 1 != 0) { + throw new IllegalArgumentException("Value [" + value + "] has a decimal part"); + } + if (value instanceof Number) { - double doubleValue = ((Number) value).doubleValue(); - if (doubleValue < Integer.MIN_VALUE || doubleValue > Integer.MAX_VALUE) { - throw new IllegalArgumentException("Value [" + value + "] is out of range for an integer"); - } - if (!coerce && doubleValue % 1 != 0) { - throw new IllegalArgumentException("Value [" + value + "] has a decimal part"); - } return ((Number) value).intValue(); } - if (value instanceof BytesRef) { - value = ((BytesRef) value).utf8ToString(); - } - return Integer.parseInt(value.toString()); + + return (int) doubleValue; } @Override @@ -612,20 +602,27 @@ public class NumberFieldMapper extends FieldMapper { LONG("long", NumericType.LONG) { @Override Long parse(Object value, boolean coerce) { + double doubleValue = objectToDouble(value); + + if (doubleValue < Long.MIN_VALUE || doubleValue > Long.MAX_VALUE) { + throw new IllegalArgumentException("Value [" + value + "] is out of range for a long"); + } + if (!coerce && doubleValue % 1 != 0) { + throw new IllegalArgumentException("Value [" + value + "] has a decimal part"); + } + if (value instanceof Number) { - double doubleValue = ((Number) value).doubleValue(); - if (doubleValue < Long.MIN_VALUE || doubleValue > Long.MAX_VALUE) { - throw new IllegalArgumentException("Value [" + value + "] is out of range for a long"); - } - if (!coerce && doubleValue % 1 != 0) { - throw new IllegalArgumentException("Value [" + value + "] has a decimal part"); - } return ((Number) value).longValue(); } - if (value instanceof BytesRef) { - value = ((BytesRef) value).utf8ToString(); + + // longs need special handling so we don't lose precision while parsing + String stringValue = (value instanceof BytesRef) ? ((BytesRef) value).utf8ToString() : value.toString(); + + try { + return Long.parseLong(stringValue); + } catch (NumberFormatException e) { + return (long) Double.parseDouble(stringValue); } - return Long.parseLong(value.toString()); } @Override @@ -781,6 +778,23 @@ public class NumberFieldMapper extends FieldMapper { return Math.signum(Double.parseDouble(value.toString())); } + /** + * Converts an Object to a double by checking it against known types first + */ + private static double objectToDouble(Object value) { + double doubleValue; + + if (value instanceof Number) { + doubleValue = ((Number) value).doubleValue(); + } else if (value instanceof BytesRef) { + doubleValue = Double.parseDouble(((BytesRef) value).utf8ToString()); + } else { + doubleValue = Double.parseDouble(value.toString()); + } + + return doubleValue; + } + } public static final class NumberFieldType extends MappedFieldType { diff --git a/core/src/test/java/org/elasticsearch/index/mapper/AbstractNumericFieldMapperTestCase.java b/core/src/test/java/org/elasticsearch/index/mapper/AbstractNumericFieldMapperTestCase.java index 57273d213b3..d2db8a50b8d 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/AbstractNumericFieldMapperTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/AbstractNumericFieldMapperTestCase.java @@ -34,6 +34,7 @@ import static org.hamcrest.Matchers.containsString; public abstract class AbstractNumericFieldMapperTestCase extends ESSingleNodeTestCase { protected Set TYPES; + protected Set WHOLE_TYPES; protected IndexService indexService; protected DocumentMapperParser parser; @@ -92,6 +93,14 @@ public abstract class AbstractNumericFieldMapperTestCase extends ESSingleNodeTes protected abstract void doTestCoerce(String type) throws IOException; + public void testDecimalCoerce() throws Exception { + for (String type : WHOLE_TYPES) { + doTestDecimalCoerce(type); + } + } + + protected abstract void doTestDecimalCoerce(String type) throws IOException; + public void testNullValue() throws IOException { for (String type : TYPES) { doTestNullValue(type); diff --git a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java index 871d62d8bd6..87aac57c16b 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -36,6 +36,7 @@ public class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase { @Override protected void setTypeList() { TYPES = new HashSet<>(Arrays.asList("byte", "short", "integer", "long", "float", "double")); + WHOLE_TYPES = new HashSet<>(Arrays.asList("byte", "short", "integer", "long")); } @Override @@ -185,6 +186,28 @@ public class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase { assertThat(e.getCause().getMessage(), containsString("passed as String")); } + @Override + protected void doTestDecimalCoerce(String type) throws IOException { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .startObject("properties").startObject("field").field("type", type).endObject().endObject() + .endObject().endObject().string(); + + DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping)); + + assertEquals(mapping, mapper.mappingSource().toString()); + + ParsedDocument doc = mapper.parse(SourceToParse.source("test", "type", "1", XContentFactory.jsonBuilder() + .startObject() + .field("field", "7.89") + .endObject() + .bytes(), + XContentType.JSON)); + + IndexableField[] fields = doc.rootDoc().getFields("field"); + IndexableField pointField = fields[0]; + assertEquals(7, pointField.numericValue().doubleValue(), 0d); + } + public void testIgnoreMalformed() throws Exception { for (String type : TYPES) { doTestIgnoreMalformed(type); @@ -301,6 +324,7 @@ public class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase { assertFalse(dvField.fieldType().stored()); } + @Override public void testEmptyName() throws IOException { // after version 5 for (String type : TYPES) { @@ -314,4 +338,5 @@ public class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase { assertThat(e.getMessage(), containsString("name cannot be empty string")); } } + } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java index e4d95441b25..8ada1dda26a 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.mapper; import com.carrotsearch.randomizedtesting.generators.RandomPicks; + import org.apache.lucene.document.Document; import org.apache.lucene.document.FloatPoint; import org.apache.lucene.document.HalfFloatPoint; @@ -35,6 +36,7 @@ import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.store.Directory; +import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.IOUtils; import org.apache.lucene.util.TestUtil; import org.elasticsearch.index.mapper.MappedFieldType.Relation; @@ -43,6 +45,7 @@ import org.hamcrest.Matchers; import org.junit.Before; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.function.Supplier; @@ -246,6 +249,37 @@ public class NumberFieldTypeTests extends FieldTypeTestCase { assertEquals(1.1d, NumberType.DOUBLE.parse(1.1, true)); } + public void testCoercions() { + assertEquals((byte) 5, NumberType.BYTE.parse((short) 5, true)); + assertEquals((byte) 5, NumberType.BYTE.parse("5", true)); + assertEquals((byte) 5, NumberType.BYTE.parse("5.0", true)); + assertEquals((byte) 5, NumberType.BYTE.parse("5.9", true)); + assertEquals((byte) 5, NumberType.BYTE.parse(new BytesRef("5.3".getBytes(StandardCharsets.UTF_8)), true)); + + assertEquals((short) 5, NumberType.SHORT.parse((byte) 5, true)); + assertEquals((short) 5, NumberType.SHORT.parse("5", true)); + assertEquals((short) 5, NumberType.SHORT.parse("5.0", true)); + assertEquals((short) 5, NumberType.SHORT.parse("5.9", true)); + assertEquals((short) 5, NumberType.SHORT.parse(new BytesRef("5.3".getBytes(StandardCharsets.UTF_8)), true)); + + assertEquals(5, NumberType.INTEGER.parse((byte) 5, true)); + assertEquals(5, NumberType.INTEGER.parse("5", true)); + assertEquals(5, NumberType.INTEGER.parse("5.0", true)); + assertEquals(5, NumberType.INTEGER.parse("5.9", true)); + assertEquals(5, NumberType.INTEGER.parse(new BytesRef("5.3".getBytes(StandardCharsets.UTF_8)), true)); + assertEquals(Integer.MAX_VALUE, NumberType.INTEGER.parse(Integer.MAX_VALUE, true)); + + assertEquals((long) 5, NumberType.LONG.parse((byte) 5, true)); + assertEquals((long) 5, NumberType.LONG.parse("5", true)); + assertEquals((long) 5, NumberType.LONG.parse("5.0", true)); + assertEquals((long) 5, NumberType.LONG.parse("5.9", true)); + assertEquals((long) 5, NumberType.LONG.parse(new BytesRef("5.3".getBytes(StandardCharsets.UTF_8)), true)); + + // these will lose precision if they get treated as a double + assertEquals(-4115420654264075766L, NumberType.LONG.parse("-4115420654264075766", true)); + assertEquals(-4115420654264075766L, NumberType.LONG.parse(-4115420654264075766L, true)); + } + public void testHalfFloatRange() throws IOException { // make sure the accuracy loss of half floats only occurs at index time // this test checks that searching half floats yields the same results as diff --git a/core/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java index 9aa62d03703..b8aff721a68 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java @@ -55,6 +55,7 @@ public class RangeFieldMapperTests extends AbstractNumericFieldMapperTestCase { @Override protected void setTypeList() { TYPES = new HashSet<>(Arrays.asList("date_range", "ip_range", "float_range", "double_range", "integer_range", "long_range")); + WHOLE_TYPES = new HashSet<>(Arrays.asList("integer_range", "long_range")); } private Object getFrom(String type) { @@ -264,6 +265,40 @@ public class RangeFieldMapperTests extends AbstractNumericFieldMapperTestCase { containsString("failed to parse date"), containsString("is not an IP string literal"))); } + @Override + protected void doTestDecimalCoerce(String type) throws IOException { + XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .startObject("properties").startObject("field").field("type", type); + + mapping = mapping.endObject().endObject().endObject().endObject(); + DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping.string())); + + assertEquals(mapping.string(), mapper.mappingSource().toString()); + + ParsedDocument doc1 = mapper.parse(SourceToParse.source("test", "type", "1", XContentFactory.jsonBuilder() + .startObject() + .startObject("field") + .field(GT_FIELD.getPreferredName(), "2.34") + .field(LT_FIELD.getPreferredName(), "5.67") + .endObject() + .endObject().bytes(), + XContentType.JSON)); + + ParsedDocument doc2 = mapper.parse(SourceToParse.source("test", "type", "1", XContentFactory.jsonBuilder() + .startObject() + .startObject("field") + .field(GT_FIELD.getPreferredName(), "2") + .field(LT_FIELD.getPreferredName(), "5") + .endObject() + .endObject().bytes(), + XContentType.JSON)); + + IndexableField[] fields1 = doc1.rootDoc().getFields("field"); + IndexableField[] fields2 = doc2.rootDoc().getFields("field"); + + assertEquals(fields1[1].binaryValue(), fields2[1].binaryValue()); + } + @Override protected void doTestNullValue(String type) throws IOException { XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type") @@ -386,4 +421,5 @@ public class RangeFieldMapperTests extends AbstractNumericFieldMapperTestCase { assertTrue(got, got.contains("\"locale\":" + "\"" + Locale.ROOT + "\"") == type.equals("date_range")); } } + } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java index ad1f3ec37fb..9fe47f4818b 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.mapper; import com.carrotsearch.randomizedtesting.generators.RandomPicks; + import org.apache.lucene.document.DoubleRange; import org.apache.lucene.document.FloatRange; import org.apache.lucene.document.InetAddressPoint; diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java index 967913f2860..682dc1777b0 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java @@ -46,19 +46,6 @@ public class GeoHashGridParserTests extends ESTestCase { assertNotNull(GeoGridAggregationBuilder.parse("geohash_grid", stParser)); } - public void testParseErrorOnNonIntPrecision() throws Exception { - XContentParser stParser = createParser(JsonXContent.jsonXContent, "{\"field\":\"my_loc\", \"precision\":\"2.0\"}"); - XContentParser.Token token = stParser.nextToken(); - assertSame(XContentParser.Token.START_OBJECT, token); - try { - GeoGridAggregationBuilder.parse("geohash_grid", stParser); - fail(); - } catch (ParsingException ex) { - assertThat(ex.getCause(), instanceOf(NumberFormatException.class)); - assertEquals("For input string: \"2.0\"", ex.getCause().getMessage()); - } - } - public void testParseErrorOnBooleanPrecision() throws Exception { XContentParser stParser = createParser(JsonXContent.jsonXContent, "{\"field\":\"my_loc\", \"precision\":false}"); XContentParser.Token token = stParser.nextToken();