honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Coerce decimal strings for whole number types by truncating the decimal part (#25835) 

This changes makes it so you can index a value like "1.0" or "1.1" into whole
number field types like byte and integer. Without this change then the above
values would have resulted in an error, even with coerce set to true.

Closes #25819
This commit is contained in:
Scott Somerville 2017-07-26 02:21:42 -04:00 committed by Adrien Grand
parent faee825fea
commit 2f8def11b5
8 changed files with 183 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java
View File

@ -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);

124
core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
View File

@ -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 {

9
core/src/test/java/org/elasticsearch/index/mapper/AbstractNumericFieldMapperTestCase.java
View File

@ -34,6 +34,7 @@ import static org.hamcrest.Matchers.containsString;
public abstract class AbstractNumericFieldMapperTestCase extends ESSingleNodeTestCase {
protected Set<String> TYPES;
protected Set<String> 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);

25
core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
View File

@ -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"));
}
}
}

34
core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java
View File

@ -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

36
core/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java
View File

@ -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"));
}
}
}

1
core/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java
View File

@ -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;

13
core/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java
View File

@ -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();