From 2d0e5c1f5d5f328a0ff857d870cb8276fce7f3c7 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Wed, 12 Jul 2017 09:47:11 +0300 Subject: [PATCH 01/11] validate half float values --- .../index/mapper/NumberFieldMapper.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) 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 ec9fcf54730e2..a1ca1f2f6c19d 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -236,6 +236,26 @@ public List createFields(String name, Number value, return fields; } + void validateParsed(Number value) throws IllegalArgumentException { + /** this checks mimics logic of {@link org.apache.lucene.document.HalfFloatPoint#halfFloatToShortBits} */ + int floatBits = Float.floatToIntBits(value.floatValue()); + int exp = (floatBits >>> 23) & 0xff; + if (exp == 0xff) { + throw new IllegalArgumentException("Infitity and NaN are not supported"); + } + if (exp == 0x00) { + return; + } + exp = exp - 127 + 15; + if (exp >= 0x1f) { + throw new IllegalArgumentException("Too large value"); + } + int shift = 23 - 10 - exp + 1; + if (exp <= 0 && shift >= 32) { + throw new IllegalArgumentException("Decimal part is too small"); + } + } + @Override FieldStats.Double stats(IndexReader reader, String fieldName, boolean isSearchable, boolean isAggregatable) throws IOException { @@ -863,6 +883,10 @@ Number valueForSearch(Number value) { return value; } + void validateParsed(Number value) throws IllegalArgumentException { + // todo override in all subclasses and then make abstract + } + /** * Returns true if the object is a number and has a decimal part */ @@ -1067,6 +1091,8 @@ protected void parseCreateField(ParseContext context, List field numericValue = fieldType().type.parse(value, coerce.value()); } + fieldType().type.validateParsed(numericValue); + if (includeInAll) { context.allEntries().addText(fieldType().name(), value.toString(), fieldType().boost()); } From 6e0a6ea344c07fc1fbee613628fc5f6e1b0a275e Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Mon, 17 Jul 2017 21:40:36 +0300 Subject: [PATCH 02/11] test upper bound for numeric mapper --- .../index/mapper/NumberFieldMapperTests.java | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) 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 871d62d8bd6be..e9da47b5588b9 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -21,6 +21,7 @@ import org.apache.lucene.index.DocValuesType; import org.apache.lucene.index.IndexableField; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; @@ -314,4 +315,43 @@ public void testEmptyName() throws IOException { assertThat(e.getMessage(), containsString("name cannot be empty string")); } } + + public void testOutOfRangeHalfFloatUpperBound() throws IOException { + DocumentMapper mapper = createDocumentMapper("half_float"); + ThrowingRunnable runnable = () -> mapper.parse(createIndexRequest("65505")); + MapperParsingException e = expectThrows(MapperParsingException.class, runnable); + assertThat(e.getCause().getMessage(), containsString("only supports finite values")); + } + + public void testOutOfRangeFloatUpperBound() throws IOException { + DocumentMapper mapper = createDocumentMapper("float"); + ThrowingRunnable runnable = () -> mapper.parse(createIndexRequest(Double.toString(Math.pow(2, 150)))); + MapperParsingException e = expectThrows(MapperParsingException.class, runnable); + assertThat(e.getCause().getMessage(), containsString("only supports finite values")); + } + + private DocumentMapper createDocumentMapper(String type) throws IOException { + String mapping = XContentFactory.jsonBuilder() + .startObject() + .startObject("type") + .startObject("properties") + .startObject("field") + .field("type", type) + .endObject() + .endObject() + .endObject() + .endObject() + .string(); + + return parser.parse("type", new CompressedXContent(mapping)); + } + + private SourceToParse createIndexRequest(String value) throws IOException { + return SourceToParse.source("test", "type", "1", XContentFactory.jsonBuilder() + .startObject() + .field("field", value) + .endObject() + .bytes(), + XContentType.JSON); + } } From d1ebd6fd57ac097095ec84acc87b8ebecf560aad Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Fri, 21 Jul 2017 10:59:39 +0300 Subject: [PATCH 03/11] test for upper bound for float, double and half_float --- .../index/mapper/NumberFieldMapper.java | 35 +++++++++++++++++++ .../index/mapper/NumberFieldMapperTests.java | 32 ++++++++++------- 2 files changed, 54 insertions(+), 13 deletions(-) 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 62aab947f9566..d78b7aa793b52 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -215,6 +215,19 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm, return query; } + @Override + void validateParsed(Number value) { + float val = value.floatValue(); + + if ( + !Float.isFinite(val) + || !Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(val))) + || val > 65504 + ) { + throw new IllegalArgumentException("[half_float] supports only finite values"); + } + } + @Override public List createFields(String name, Number value, boolean indexed, boolean docValued, boolean stored) { @@ -292,6 +305,13 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm, return query; } + @Override + void validateParsed(Number value) { + if (!Float.isFinite(value.floatValue())) { + throw new IllegalArgumentException("[float] supports only finite values"); + } + } + @Override public List createFields(String name, Number value, boolean indexed, boolean docValued, boolean stored) { @@ -369,6 +389,13 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm, return query; } + @Override + void validateParsed(Number value) { + if (!Double.isFinite(value.doubleValue())) { + throw new IllegalArgumentException("[double] supports only finite values, but got [" + value.toString() + "]"); + } + } + @Override public List createFields(String name, Number value, boolean indexed, boolean docValued, boolean stored) { @@ -750,6 +777,12 @@ Number valueForSearch(Number value) { return value; } + /** + * @throws IllegalArgumentException if value is not finite for this type + */ + void validateParsed(Number value) { + } + /** * Returns true if the object is a number and has a decimal part */ @@ -949,6 +982,8 @@ protected void parseCreateField(ParseContext context, List field numericValue = fieldType().type.parse(value, coerce.value()); } + fieldType().type.validateParsed(numericValue); + if (includeInAll) { context.allEntries().addText(fieldType().name(), value.toString(), fieldType().boost()); } 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 e9da47b5588b9..9040409a94381 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -21,14 +21,16 @@ import org.apache.lucene.index.DocValuesType; import org.apache.lucene.index.IndexableField; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; +import java.math.BigDecimal; import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import static org.hamcrest.Matchers.containsString; @@ -316,20 +318,24 @@ public void testEmptyName() throws IOException { } } - public void testOutOfRangeHalfFloatUpperBound() throws IOException { - DocumentMapper mapper = createDocumentMapper("half_float"); - ThrowingRunnable runnable = () -> mapper.parse(createIndexRequest("65505")); - MapperParsingException e = expectThrows(MapperParsingException.class, runnable); - assertThat(e.getCause().getMessage(), containsString("only supports finite values")); + public void testOutOfRangeValue() { + final Map outOfRangeValues = new HashMap<>(); + outOfRangeValues.put("float", new BigDecimal("3.4028235E39").toString()); + outOfRangeValues.put("double", new BigDecimal("1.7976931348623157E309").toString()); + outOfRangeValues.put("half_float", new BigDecimal("65504.1").toString()); + + outOfRangeValues.forEach((type, value) -> { + try { + createDocumentMapper(type).parse(createIndexRequest(value)); + fail("Mapper parsing exception expected for [" + type + "]"); + } catch (MapperParsingException e) { + assertThat(e.getCause().getMessage(), containsString("[" + type + "] supports only finite values")); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); } - public void testOutOfRangeFloatUpperBound() throws IOException { - DocumentMapper mapper = createDocumentMapper("float"); - ThrowingRunnable runnable = () -> mapper.parse(createIndexRequest(Double.toString(Math.pow(2, 150)))); - MapperParsingException e = expectThrows(MapperParsingException.class, runnable); - assertThat(e.getCause().getMessage(), containsString("only supports finite values")); - } - private DocumentMapper createDocumentMapper(String type) throws IOException { String mapping = XContentFactory.jsonBuilder() .startObject() From 44983d8a35cf7124a33c886e1742c5eb0255f704 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 23 Jul 2017 10:07:23 +0300 Subject: [PATCH 04/11] more tests on NaN and Infinity for NumberFieldMapper --- .../index/mapper/NumberFieldMapper.java | 112 ++++++++++-------- .../index/mapper/NumberFieldMapperTests.java | 109 +++++++++++++---- 2 files changed, 149 insertions(+), 72 deletions(-) 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 d78b7aa793b52..76947079a2184 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -162,12 +162,25 @@ public enum NumberType { HALF_FLOAT("half_float", NumericType.HALF_FLOAT) { @Override Float parse(Object value, boolean coerce) { - return (Float) FLOAT.parse(value, false); + final Float result; + + if (value instanceof Number) { + result = ((Number) value).floatValue(); + } else { + if (value instanceof BytesRef) { + value = ((BytesRef) value).utf8ToString(); + } + result = Float.parseFloat(value.toString()); + } + validateParsed(result); + return result; } @Override Float parse(XContentParser parser, boolean coerce) throws IOException { - return parser.floatValue(coerce); + Float parsed = parser.floatValue(coerce); + validateParsed(parsed); + return parsed; } @Override @@ -215,19 +228,6 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm, return query; } - @Override - void validateParsed(Number value) { - float val = value.floatValue(); - - if ( - !Float.isFinite(val) - || !Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(val))) - || val > 65504 - ) { - throw new IllegalArgumentException("[half_float] supports only finite values"); - } - } - @Override public List createFields(String name, Number value, boolean indexed, boolean docValued, boolean stored) { @@ -244,22 +244,39 @@ public List createFields(String name, Number value, } return fields; } + + private void validateParsed(Float value) { + if ( + value.isNaN() || value.isInfinite() + || value > 65504 + || !Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(value))) + ) { + throw new IllegalArgumentException("[half_float] supports only finite values, but [" + value + "] given"); + } + } }, FLOAT("float", NumericType.FLOAT) { @Override Float parse(Object value, boolean coerce) { + final Float result; + if (value instanceof Number) { - return ((Number) value).floatValue(); - } - if (value instanceof BytesRef) { - value = ((BytesRef) value).utf8ToString(); + result = ((Number) value).floatValue(); + } else { + if (value instanceof BytesRef) { + value = ((BytesRef) value).utf8ToString(); + } + result = Float.parseFloat(value.toString()); } - return Float.parseFloat(value.toString()); + validateParsed(result); + return result; } @Override Float parse(XContentParser parser, boolean coerce) throws IOException { - return parser.floatValue(coerce); + Float parsed = parser.floatValue(coerce); + validateParsed(parsed); + return parsed; } @Override @@ -305,13 +322,6 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm, return query; } - @Override - void validateParsed(Number value) { - if (!Float.isFinite(value.floatValue())) { - throw new IllegalArgumentException("[float] supports only finite values"); - } - } - @Override public List createFields(String name, Number value, boolean indexed, boolean docValued, boolean stored) { @@ -328,22 +338,35 @@ public List createFields(String name, Number value, } return fields; } + + private void validateParsed(Float value) { + if (value.isInfinite() || value.isNaN()) { + throw new IllegalArgumentException("[float] supports only finite values, but [" + value + "] given"); + } + } }, DOUBLE("double", NumericType.DOUBLE) { @Override Double parse(Object value, boolean coerce) { + final Double result; + if (value instanceof Number) { - return ((Number) value).doubleValue(); - } - if (value instanceof BytesRef) { - value = ((BytesRef) value).utf8ToString(); + result = ((Number) value).doubleValue(); + } else { + if (value instanceof BytesRef) { + value = ((BytesRef) value).utf8ToString(); + } + result = Double.parseDouble(value.toString()); } - return Double.parseDouble(value.toString()); + validateParsed(result); + return result; } @Override Double parse(XContentParser parser, boolean coerce) throws IOException { - return parser.doubleValue(coerce); + Double parsed = parser.doubleValue(coerce); + validateParsed(parsed); + return parsed; } @Override @@ -389,13 +412,6 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm, return query; } - @Override - void validateParsed(Number value) { - if (!Double.isFinite(value.doubleValue())) { - throw new IllegalArgumentException("[double] supports only finite values, but got [" + value.toString() + "]"); - } - } - @Override public List createFields(String name, Number value, boolean indexed, boolean docValued, boolean stored) { @@ -412,6 +428,12 @@ public List createFields(String name, Number value, } return fields; } + + private void validateParsed(Double value) { + if (value.isInfinite() || value.isNaN()) { + throw new IllegalArgumentException("[double] supports only finite values, but [" + value + "] given"); + } + } }, BYTE("byte", NumericType.BYTE) { @Override @@ -777,12 +799,6 @@ Number valueForSearch(Number value) { return value; } - /** - * @throws IllegalArgumentException if value is not finite for this type - */ - void validateParsed(Number value) { - } - /** * Returns true if the object is a number and has a decimal part */ @@ -982,8 +998,6 @@ protected void parseCreateField(ParseContext context, List field numericValue = fieldType().type.parse(value, coerce.value()); } - fieldType().type.validateParsed(numericValue); - if (includeInAll) { context.allEntries().addText(fieldType().name(), value.toString(), fieldType().boost()); } 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 9040409a94381..16ceaf133181d 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -21,16 +21,15 @@ import org.apache.lucene.index.DocValuesType; import org.apache.lucene.index.IndexableField; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; import java.math.BigDecimal; -import java.util.Arrays; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; +import java.math.BigInteger; +import java.util.*; import static org.hamcrest.Matchers.containsString; @@ -318,22 +317,91 @@ public void testEmptyName() throws IOException { } } - public void testOutOfRangeValue() { - final Map outOfRangeValues = new HashMap<>(); - outOfRangeValues.put("float", new BigDecimal("3.4028235E39").toString()); - outOfRangeValues.put("double", new BigDecimal("1.7976931348623157E309").toString()); - outOfRangeValues.put("half_float", new BigDecimal("65504.1").toString()); + public void testOutOfRangeValuesFromRequest() throws IOException { + final List> inputs = Arrays.asList( + new Tuple<>("half_float", "65504.1", "[half_float] supports only finite values"), + new Tuple<>("float", "3.4028235E39", "[float] supports only finite values"), + new Tuple<>("double", "1.7976931348623157E309", "[double] supports only finite values"), - outOfRangeValues.forEach((type, value) -> { + new Tuple<>("byte", "128", "is out of range for a byte"), + new Tuple<>("short", "32768", "is out of range for a short"), + new Tuple<>("integer", "2147483648", "For input string"), + new Tuple<>("long", "92233720368547758080", "For input string"), + + new Tuple<>("half_float", Float.NaN, "[half_float] supports only finite values"), + new Tuple<>("float", Float.NaN, "[float] supports only finite values"), + new Tuple<>("double", Double.NaN, "[double] supports only finite values"), + + new Tuple<>("half_float", Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), + new Tuple<>("float", Float.POSITIVE_INFINITY, "[float] supports only finite values"), + new Tuple<>("double", Double.POSITIVE_INFINITY, "[double] supports only finite values") + ); + + for(Tuple item: inputs) { try { - createDocumentMapper(type).parse(createIndexRequest(value)); - fail("Mapper parsing exception expected for [" + type + "]"); + parseRequest(item.type, createIndexRequest(item.value)); + fail("Mapper parsing exception expected for [" + item.type + "] with value [" + item.value + "]"); } catch (MapperParsingException e) { - assertThat(e.getCause().getMessage(), containsString("[" + type + "] supports only finite values")); - } catch (IOException e) { - throw new RuntimeException(e); + assertThat("Incorrect error message for [" + item.type + "] with value [" + item.value + "]", + e.getCause().getMessage(), containsString(item.message)); + } + } + } + + public void testOutOfRangeValuesFromObject() throws IOException { + final List> inputs = Arrays.asList( + new Tuple<>(NumberFieldMapper.NumberType.BYTE, "128", "Value out of range"), + new Tuple<>(NumberFieldMapper.NumberType.SHORT, "32768", "Value out of range"), + new Tuple<>(NumberFieldMapper.NumberType.INTEGER, "2147483648", "For input string"), + new Tuple<>(NumberFieldMapper.NumberType.LONG, "9223372036854775808", "For input string"), + + new Tuple<>(NumberFieldMapper.NumberType.BYTE, 128, "is out of range for a byte"), + new Tuple<>(NumberFieldMapper.NumberType.SHORT, 32768, "is out of range for a short"), + new Tuple<>(NumberFieldMapper.NumberType.INTEGER, 2147483648L, "is out of range for an integer"), + new Tuple<>(NumberFieldMapper.NumberType.LONG, new BigInteger("92233720368547758080"), " is out of range for a long"), + + new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), + + new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, 65504.1, "[half_float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.FLOAT, 3.4028235E39, "[float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"), + + new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.FLOAT, Float.NaN, "[float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"), + + new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values") + ); + + for (Tuple item: inputs) { + try { + item.type.parse(item.value, false); + fail("Mapper parsing exception expected for [" + item.type + "] with value [" + item.value + "]"); + } catch (IllegalArgumentException e) { + assertThat("Incorrect error message for [" + item.type + "] with value [" + item.value + "]", + e.getMessage(), containsString(item.message)); } - }); + } + } + + private static class Tuple { + final K type; + final V value; + final E message; + + Tuple(K type, V value, E message) { + this.type = type; + this.value = value; + this.message = message; + } + } + + private void parseRequest(String type, BytesReference content) throws IOException { + createDocumentMapper(type).parse(SourceToParse.source("test", "type", "1", content, XContentType.JSON)); } private DocumentMapper createDocumentMapper(String type) throws IOException { @@ -352,12 +420,7 @@ private DocumentMapper createDocumentMapper(String type) throws IOException { return parser.parse("type", new CompressedXContent(mapping)); } - private SourceToParse createIndexRequest(String value) throws IOException { - return SourceToParse.source("test", "type", "1", XContentFactory.jsonBuilder() - .startObject() - .field("field", value) - .endObject() - .bytes(), - XContentType.JSON); + private BytesReference createIndexRequest(Object value) throws IOException { + return XContentFactory.jsonBuilder().startObject().field("field", value).endObject().bytes(); } } From ecf34245aa288bc1d8ac362f3c8e3bbcdea78598 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 23 Jul 2017 11:41:37 +0300 Subject: [PATCH 05/11] fix checkstyle errors --- .../index/mapper/NumberFieldMapperTests.java | 45 ++------------ .../index/mapper/NumberFieldTypeTests.java | 61 ++++++++++++++++++- 2 files changed, 63 insertions(+), 43 deletions(-) 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 16ceaf133181d..71a42c5b02dcd 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -29,7 +29,10 @@ import java.io.IOException; import java.math.BigDecimal; import java.math.BigInteger; -import java.util.*; +import java.util.List; +import java.util.Arrays; +import java.util.HashSet; + import static org.hamcrest.Matchers.containsString; @@ -348,46 +351,6 @@ public void testOutOfRangeValuesFromRequest() throws IOException { } } - public void testOutOfRangeValuesFromObject() throws IOException { - final List> inputs = Arrays.asList( - new Tuple<>(NumberFieldMapper.NumberType.BYTE, "128", "Value out of range"), - new Tuple<>(NumberFieldMapper.NumberType.SHORT, "32768", "Value out of range"), - new Tuple<>(NumberFieldMapper.NumberType.INTEGER, "2147483648", "For input string"), - new Tuple<>(NumberFieldMapper.NumberType.LONG, "9223372036854775808", "For input string"), - - new Tuple<>(NumberFieldMapper.NumberType.BYTE, 128, "is out of range for a byte"), - new Tuple<>(NumberFieldMapper.NumberType.SHORT, 32768, "is out of range for a short"), - new Tuple<>(NumberFieldMapper.NumberType.INTEGER, 2147483648L, "is out of range for an integer"), - new Tuple<>(NumberFieldMapper.NumberType.LONG, new BigInteger("92233720368547758080"), " is out of range for a long"), - - new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), - - new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, 65504.1, "[half_float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.FLOAT, 3.4028235E39, "[float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"), - - new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.FLOAT, Float.NaN, "[float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"), - - new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values") - ); - - for (Tuple item: inputs) { - try { - item.type.parse(item.value, false); - fail("Mapper parsing exception expected for [" + item.type + "] with value [" + item.value + "]"); - } catch (IllegalArgumentException e) { - assertThat("Incorrect error message for [" + item.type + "] with value [" + item.value + "]", - e.getMessage(), containsString(item.message)); - } - } - } - private static class Tuple { final K type; final V value; 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 e4d95441b2573..a03d83c1ee23c 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -43,9 +43,14 @@ import org.junit.Before; import java.io.IOException; +import java.math.BigDecimal; +import java.math.BigInteger; import java.util.Arrays; +import java.util.List; import java.util.function.Supplier; +import static org.hamcrest.Matchers.containsString; + public class NumberFieldTypeTests extends FieldTypeTestCase { NumberType type; @@ -266,8 +271,8 @@ public void testHalfFloatRange() throws IOException { IndexSearcher searcher = newSearcher(reader); final int numQueries = 1000; for (int i = 0; i < numQueries; ++i) { - float l = (randomFloat() * 2 - 1) * 70000; - float u = (randomFloat() * 2 - 1) * 70000; + float l = (randomFloat() * 2 - 1) * 65504; + float u = (randomFloat() * 2 - 1) * 65504; boolean includeLower = randomBoolean(); boolean includeUpper = randomBoolean(); Query floatQ = NumberFieldMapper.NumberType.FLOAT.rangeQuery("float", l, u, includeLower, includeUpper, false); @@ -348,4 +353,56 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier valueSu reader.close(); dir.close(); } + + public void testParseOutOfRangeValues() throws IOException { + final List> inputs = Arrays.asList( + new Tuple<>(NumberType.BYTE, "128", "Value out of range"), + new Tuple<>(NumberType.SHORT, "32768", "Value out of range"), + new Tuple<>(NumberType.INTEGER, "2147483648", "For input string"), + new Tuple<>(NumberType.LONG, "9223372036854775808", "For input string"), + + new Tuple<>(NumberType.BYTE, 128, "is out of range for a byte"), + new Tuple<>(NumberType.SHORT, 32768, "is out of range for a short"), + new Tuple<>(NumberType.INTEGER, 2147483648L, "is out of range for an integer"), + new Tuple<>(NumberType.LONG, new BigInteger("92233720368547758080"), " is out of range for a long"), + + new Tuple<>(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), + new Tuple<>(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), + new Tuple<>(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), + + new Tuple<>(NumberType.HALF_FLOAT, 65504.1, "[half_float] supports only finite values"), + new Tuple<>(NumberType.FLOAT, 3.4028235E39, "[float] supports only finite values"), + new Tuple<>(NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"), + + new Tuple<>(NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), + new Tuple<>(NumberType.FLOAT, Float.NaN, "[float] supports only finite values"), + new Tuple<>(NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"), + + new Tuple<>(NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), + new Tuple<>(NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"), + new Tuple<>(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values") + ); + + for (Tuple item: inputs) { + try { + item.type.parse(item.value, false); + fail("Parsing exception expected for [" + item.type + "] with value [" + item.value + "]"); + } catch (IllegalArgumentException e) { + assertThat("Incorrect error message for [" + item.type + "] with value [" + item.value + "]", + e.getMessage(), containsString(item.message)); + } + } + } + + private static class Tuple { + final K type; + final V value; + final E message; + + Tuple(K type, V value, E message) { + this.type = type; + this.value = value; + this.message = message; + } + } } From b5d231d1be6bbd97439956a0554b6584bdd91520 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 23 Jul 2017 20:19:15 +0300 Subject: [PATCH 06/11] minor renaming --- .../index/mapper/NumberFieldMapper.java | 6 +- .../index/mapper/NumberFieldMapperTests.java | 52 +++++++------- .../index/mapper/NumberFieldTypeTests.java | 71 ++++++++++--------- 3 files changed, 68 insertions(+), 61 deletions(-) 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 76947079a2184..5fa62d599e9fb 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -251,7 +251,7 @@ private void validateParsed(Float value) { || value > 65504 || !Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(value))) ) { - throw new IllegalArgumentException("[half_float] supports only finite values, but [" + value + "] given"); + throw new IllegalArgumentException("[half_float] supports only finite values, but got [" + value + "]"); } } }, @@ -341,7 +341,7 @@ public List createFields(String name, Number value, private void validateParsed(Float value) { if (value.isInfinite() || value.isNaN()) { - throw new IllegalArgumentException("[float] supports only finite values, but [" + value + "] given"); + throw new IllegalArgumentException("[float] supports only finite values, but got [" + value + "]"); } } }, @@ -431,7 +431,7 @@ public List createFields(String name, Number value, private void validateParsed(Double value) { if (value.isInfinite() || value.isNaN()) { - throw new IllegalArgumentException("[double] supports only finite values, but [" + value + "] given"); + throw new IllegalArgumentException("[double] supports only finite values, but got [" + value + "]"); } } }, 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 71a42c5b02dcd..ffe6e7386774e 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -27,13 +27,10 @@ import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; -import java.math.BigDecimal; -import java.math.BigInteger; import java.util.List; import java.util.Arrays; import java.util.HashSet; - import static org.hamcrest.Matchers.containsString; public class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase { @@ -320,27 +317,27 @@ public void testEmptyName() throws IOException { } } - public void testOutOfRangeValuesFromRequest() throws IOException { - final List> inputs = Arrays.asList( - new Tuple<>("half_float", "65504.1", "[half_float] supports only finite values"), - new Tuple<>("float", "3.4028235E39", "[float] supports only finite values"), - new Tuple<>("double", "1.7976931348623157E309", "[double] supports only finite values"), + public void testOutOfRangeValues() throws IOException { + final List> inputs = Arrays.asList( + Triple.of("byte", "128", "is out of range for a byte"), + Triple.of("short", "32768", "is out of range for a short"), + Triple.of("integer", "2147483648", "For input string"), + Triple.of("long", "92233720368547758080", "For input string"), - new Tuple<>("byte", "128", "is out of range for a byte"), - new Tuple<>("short", "32768", "is out of range for a short"), - new Tuple<>("integer", "2147483648", "For input string"), - new Tuple<>("long", "92233720368547758080", "For input string"), + Triple.of("half_float", "65504.1", "[half_float] supports only finite values"), + Triple.of("float", "3.4028235E39", "[float] supports only finite values"), + Triple.of("double", "1.7976931348623157E309", "[double] supports only finite values"), - new Tuple<>("half_float", Float.NaN, "[half_float] supports only finite values"), - new Tuple<>("float", Float.NaN, "[float] supports only finite values"), - new Tuple<>("double", Double.NaN, "[double] supports only finite values"), + Triple.of("half_float", Float.NaN, "[half_float] supports only finite values"), + Triple.of("float", Float.NaN, "[float] supports only finite values"), + Triple.of("double", Double.NaN, "[double] supports only finite values"), - new Tuple<>("half_float", Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), - new Tuple<>("float", Float.POSITIVE_INFINITY, "[float] supports only finite values"), - new Tuple<>("double", Double.POSITIVE_INFINITY, "[double] supports only finite values") + Triple.of("half_float", Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), + Triple.of("float", Float.POSITIVE_INFINITY, "[float] supports only finite values"), + Triple.of("double", Double.POSITIVE_INFINITY, "[double] supports only finite values") ); - for(Tuple item: inputs) { + for(Triple item: inputs) { try { parseRequest(item.type, createIndexRequest(item.value)); fail("Mapper parsing exception expected for [" + item.type + "] with value [" + item.value + "]"); @@ -351,15 +348,20 @@ public void testOutOfRangeValuesFromRequest() throws IOException { } } - private static class Tuple { + private static class Triple { + final K type; final V value; - final E message; + final M message; + + static Triple of(K t, V v, M m) { + return new Triple<>(t, v, m); + } - Tuple(K type, V value, E message) { - this.type = type; - this.value = value; - this.message = message; + Triple(K t, V v, M m) { + type = t; + value = v; + message = m; } } 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 a03d83c1ee23c..603255a8137ce 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -355,35 +355,35 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier valueSu } public void testParseOutOfRangeValues() throws IOException { - final List> inputs = Arrays.asList( - new Tuple<>(NumberType.BYTE, "128", "Value out of range"), - new Tuple<>(NumberType.SHORT, "32768", "Value out of range"), - new Tuple<>(NumberType.INTEGER, "2147483648", "For input string"), - new Tuple<>(NumberType.LONG, "9223372036854775808", "For input string"), - - new Tuple<>(NumberType.BYTE, 128, "is out of range for a byte"), - new Tuple<>(NumberType.SHORT, 32768, "is out of range for a short"), - new Tuple<>(NumberType.INTEGER, 2147483648L, "is out of range for an integer"), - new Tuple<>(NumberType.LONG, new BigInteger("92233720368547758080"), " is out of range for a long"), - - new Tuple<>(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), - new Tuple<>(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), - new Tuple<>(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), - - new Tuple<>(NumberType.HALF_FLOAT, 65504.1, "[half_float] supports only finite values"), - new Tuple<>(NumberType.FLOAT, 3.4028235E39, "[float] supports only finite values"), - new Tuple<>(NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"), - - new Tuple<>(NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), - new Tuple<>(NumberType.FLOAT, Float.NaN, "[float] supports only finite values"), - new Tuple<>(NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"), - - new Tuple<>(NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), - new Tuple<>(NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"), - new Tuple<>(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values") + final List> inputs = Arrays.asList( + Triple.of(NumberType.BYTE, "128", "Value out of range"), + Triple.of(NumberType.SHORT, "32768", "Value out of range"), + Triple.of(NumberType.INTEGER, "2147483648", "For input string"), + Triple.of(NumberType.LONG, "9223372036854775808", "For input string"), + + Triple.of(NumberType.BYTE, 128, "is out of range for a byte"), + Triple.of(NumberType.SHORT, 32768, "is out of range for a short"), + Triple.of(NumberType.INTEGER, 2147483648L, "is out of range for an integer"), + Triple.of(NumberType.LONG, new BigInteger("92233720368547758080"), " is out of range for a long"), + + Triple.of(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), + Triple.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), + Triple.of(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), + + Triple.of(NumberType.HALF_FLOAT, 65504.1, "[half_float] supports only finite values"), + Triple.of(NumberType.FLOAT, 3.4028235E39, "[float] supports only finite values"), + Triple.of(NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"), + + Triple.of(NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), + Triple.of(NumberType.FLOAT, Float.NaN, "[float] supports only finite values"), + Triple.of(NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"), + + Triple.of(NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), + Triple.of(NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"), + Triple.of(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values") ); - for (Tuple item: inputs) { + for (Triple item: inputs) { try { item.type.parse(item.value, false); fail("Parsing exception expected for [" + item.type + "] with value [" + item.value + "]"); @@ -394,15 +394,20 @@ public void testParseOutOfRangeValues() throws IOException { } } - private static class Tuple { + private static class Triple { + final K type; final V value; - final E message; + final M message; + + static Triple of(K t, V v, M m) { + return new Triple<>(t, v, m); + } - Tuple(K type, V value, E message) { - this.type = type; - this.value = value; - this.message = message; + Triple(K t, V v, M m) { + type = t; + value = v; + message = m; } } } From 7423c4df4398d63ff098e591cb6a3cd5b7968172 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Thu, 27 Jul 2017 15:14:21 +0300 Subject: [PATCH 07/11] comments for disabled test --- .../org/elasticsearch/index/mapper/NumberFieldMapperTests.java | 3 ++- .../org/elasticsearch/index/mapper/NumberFieldTypeTests.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) 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 da2150421e2d5..522bc3d4544c7 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -345,10 +345,11 @@ public void testEmptyName() throws IOException { public void testOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( + // TODO fix min/max value validation for short, integer and long and uncomment tests OutOfRangeSpec.of(NumberType.BYTE, "128", "is out of range for a byte"), //OutOfRangeSpec.of(NumberType.SHORT, "32768", "is out of range for a short"), //OutOfRangeSpec.of(NumberType.INTEGER, "2147483648", "For input string"), - OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "For input string"), + //OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "For input string"), OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), 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 fdab23232e593..dce69243c3bac 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -390,8 +390,9 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier valueSu public void testParseOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( + // TODO fix min/max value validation for long and uncomment tests OutOfRangeSpec.of(NumberType.BYTE, "128", "out of range for a byte"), - OutOfRangeSpec.of(NumberType.SHORT, "32768", "out of range for a short"), //@todo fix this value + OutOfRangeSpec.of(NumberType.SHORT, "32768", "out of range for a short"), OutOfRangeSpec.of(NumberType.INTEGER, "2147483648", "out of range for an integer"), //OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "out of range for a long"), From 11ce7c86114735f58a6ac9b1e4115171bbc75825 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 30 Jul 2017 07:14:26 +0300 Subject: [PATCH 08/11] tests for byte/short/integer/long removed and will be added in separate PR --- .../index/mapper/NumberFieldMapperTests.java | 6 ------ .../index/mapper/NumberFieldTypeTests.java | 11 ----------- 2 files changed, 17 deletions(-) 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 522bc3d4544c7..f7e791cfb1c2c 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -345,12 +345,6 @@ public void testEmptyName() throws IOException { public void testOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( - // TODO fix min/max value validation for short, integer and long and uncomment tests - OutOfRangeSpec.of(NumberType.BYTE, "128", "is out of range for a byte"), - //OutOfRangeSpec.of(NumberType.SHORT, "32768", "is out of range for a short"), - //OutOfRangeSpec.of(NumberType.INTEGER, "2147483648", "For input string"), - //OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "For input string"), - OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), OutOfRangeSpec.of(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), 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 dce69243c3bac..5c534646aa131 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -390,17 +390,6 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier valueSu public void testParseOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( - // TODO fix min/max value validation for long and uncomment tests - OutOfRangeSpec.of(NumberType.BYTE, "128", "out of range for a byte"), - OutOfRangeSpec.of(NumberType.SHORT, "32768", "out of range for a short"), - OutOfRangeSpec.of(NumberType.INTEGER, "2147483648", "out of range for an integer"), - //OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "out of range for a long"), - - OutOfRangeSpec.of(NumberType.BYTE, 128, "is out of range for a byte"), - OutOfRangeSpec.of(NumberType.SHORT, 327684, "is out of range for a short"), - OutOfRangeSpec.of(NumberType.INTEGER, 2147483648L, "is out of range for an integer"), - //OutOfRangeSpec.of(NumberType.LONG, new BigInteger("9223372036854775808"), " is out of range for a long"), - OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), OutOfRangeSpec.of(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), From 3902911f2a11051156735bcfffd8146b7e30eda0 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 30 Jul 2017 07:16:07 +0300 Subject: [PATCH 09/11] remove unused import --- .../org/elasticsearch/index/mapper/NumberFieldTypeTests.java | 1 - 1 file changed, 1 deletion(-) 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 5c534646aa131..66db603077c52 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -46,7 +46,6 @@ import java.io.IOException; import java.math.BigDecimal; -import java.math.BigInteger; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; From cd38455a493c4c95eb8037b51f642aa37f888464 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Fri, 4 Aug 2017 10:54:59 +0300 Subject: [PATCH 10/11] Fix scaledfloat out of range validation message --- .../index/mapper/ScaledFloatFieldMapper.java | 42 +++++++++++++++---- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java index 3cc02584b12fe..943e8aa08a169 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java @@ -28,6 +28,7 @@ import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.SortField; +import org.apache.lucene.util.BytesRef; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.settings.Setting; @@ -144,7 +145,7 @@ public Mapper.Builder parse(String name, Map node, if (propNode == null) { throw new MapperParsingException("Property [null_value] cannot be null."); } - builder.nullValue(NumberFieldMapper.NumberType.DOUBLE.parse(propNode, false)); + builder.nullValue(ScaledFloatFieldMapper.parse(propNode)); iterator.remove(); } else if (propName.equals("ignore_malformed")) { builder.ignoreMalformed(TypeParsers.nodeBooleanValue(name, "ignore_malformed", propNode, parserContext)); @@ -153,7 +154,7 @@ public Mapper.Builder parse(String name, Map node, builder.coerce(TypeParsers.nodeBooleanValue(name, "coerce", propNode, parserContext)); iterator.remove(); } else if (propName.equals("scaling_factor")) { - builder.scalingFactor(NumberFieldMapper.NumberType.DOUBLE.parse(propNode, false).doubleValue()); + builder.scalingFactor(ScaledFloatFieldMapper.parse(propNode)); iterator.remove(); } } @@ -207,7 +208,7 @@ public void checkCompatibility(MappedFieldType other, List conflicts, bo @Override public Query termQuery(Object value, QueryShardContext context) { failIfNotIndexed(); - double queryValue = NumberFieldMapper.NumberType.DOUBLE.parse(value, false).doubleValue(); + double queryValue = parse(value); long scaledValue = Math.round(queryValue * scalingFactor); Query query = NumberFieldMapper.NumberType.LONG.termQuery(name(), scaledValue); if (boost() != 1f) { @@ -221,7 +222,7 @@ public Query termsQuery(List values, QueryShardContext context) { failIfNotIndexed(); List scaledValues = new ArrayList<>(values.size()); for (Object value : values) { - double queryValue = NumberFieldMapper.NumberType.DOUBLE.parse(value, false).doubleValue(); + double queryValue = parse(value); long scaledValue = Math.round(queryValue * scalingFactor); scaledValues.add(scaledValue); } @@ -237,7 +238,7 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower failIfNotIndexed(); Long lo = null; if (lowerTerm != null) { - double dValue = NumberFieldMapper.NumberType.DOUBLE.parse(lowerTerm, false).doubleValue(); + double dValue = parse(lowerTerm); if (includeLower == false) { dValue = Math.nextUp(dValue); } @@ -245,7 +246,7 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower } Long hi = null; if (upperTerm != null) { - double dValue = NumberFieldMapper.NumberType.DOUBLE.parse(upperTerm, false).doubleValue(); + double dValue = parse(upperTerm); if (includeUpper == false) { dValue = Math.nextDown(dValue); } @@ -366,7 +367,7 @@ protected void parseCreateField(ParseContext context, List field value = null; } else { try { - numericValue = NumberFieldMapper.NumberType.DOUBLE.parse(parser, coerce.value()); + numericValue = parse(parser, coerce.value()); } catch (IllegalArgumentException e) { if (ignoreMalformed.value()) { return; @@ -390,7 +391,7 @@ protected void parseCreateField(ParseContext context, List field } if (numericValue == null) { - numericValue = NumberFieldMapper.NumberType.DOUBLE.parse(value, false); + numericValue = parse(value); } if (includeInAll) { @@ -451,6 +452,31 @@ protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, } } + static Double parse(Object value) { + return objectToDouble(value); + } + + private static Double parse(XContentParser parser, boolean coerce) throws IOException { + return parser.doubleValue(coerce); + } + + /** + * 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; + } + private static class ScaledFloatIndexFieldData implements IndexNumericFieldData { private final IndexNumericFieldData scaledFieldData; From 3c666a9335ba3ab392f2327afa2fa06799307b94 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Tue, 8 Aug 2017 09:39:18 +0300 Subject: [PATCH 11/11] 1) delayed autoboxing in numbertype.parse(...) 2) no redudant checks in half_float validation 3) tests with negative values for half_float/float/double --- .../index/mapper/NumberFieldMapper.java | 28 ++++++++----------- .../index/mapper/NumberFieldMapperTests.java | 12 ++++++-- .../index/mapper/NumberFieldTypeTests.java | 16 ++++++++--- 3 files changed, 34 insertions(+), 22 deletions(-) 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 5bbcd4794e1d2..2ecdc1230f07b 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -162,7 +162,7 @@ public enum NumberType { HALF_FLOAT("half_float", NumericType.HALF_FLOAT) { @Override Float parse(Object value, boolean coerce) { - final Float result; + final float result; if (value instanceof Number) { result = ((Number) value).floatValue(); @@ -178,7 +178,7 @@ Float parse(Object value, boolean coerce) { @Override Float parse(XContentParser parser, boolean coerce) throws IOException { - Float parsed = parser.floatValue(coerce); + float parsed = parser.floatValue(coerce); validateParsed(parsed); return parsed; } @@ -245,12 +245,8 @@ public List createFields(String name, Number value, return fields; } - private void validateParsed(Float value) { - if ( - value.isNaN() || value.isInfinite() - || value > 65504 - || !Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(value))) - ) { + private void validateParsed(float value) { + if (!Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(value)))) { throw new IllegalArgumentException("[half_float] supports only finite values, but got [" + value + "]"); } } @@ -258,7 +254,7 @@ private void validateParsed(Float value) { FLOAT("float", NumericType.FLOAT) { @Override Float parse(Object value, boolean coerce) { - final Float result; + final float result; if (value instanceof Number) { result = ((Number) value).floatValue(); @@ -274,7 +270,7 @@ Float parse(Object value, boolean coerce) { @Override Float parse(XContentParser parser, boolean coerce) throws IOException { - Float parsed = parser.floatValue(coerce); + float parsed = parser.floatValue(coerce); validateParsed(parsed); return parsed; } @@ -339,8 +335,8 @@ public List createFields(String name, Number value, return fields; } - private void validateParsed(Float value) { - if (value.isInfinite() || value.isNaN()) { + private void validateParsed(float value) { + if (!Float.isFinite(value)) { throw new IllegalArgumentException("[float] supports only finite values, but got [" + value + "]"); } } @@ -348,14 +344,14 @@ private void validateParsed(Float value) { DOUBLE("double", NumericType.DOUBLE) { @Override Double parse(Object value, boolean coerce) { - Double parsed = objectToDouble(value); + double parsed = objectToDouble(value); validateParsed(parsed); return parsed; } @Override Double parse(XContentParser parser, boolean coerce) throws IOException { - Double parsed = parser.doubleValue(coerce); + double parsed = parser.doubleValue(coerce); validateParsed(parsed); return parsed; } @@ -420,8 +416,8 @@ public List createFields(String name, Number value, return fields; } - private void validateParsed(Double value) { - if (value.isInfinite() || value.isNaN()) { + private void validateParsed(double value) { + if (!Double.isFinite(value)) { throw new IllegalArgumentException("[double] supports only finite values, but got [" + value + "]"); } } 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 f7e791cfb1c2c..3ace8c0451320 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -345,17 +345,25 @@ public void testEmptyName() throws IOException { public void testOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( - OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), + OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65520", "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), OutOfRangeSpec.of(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), + OutOfRangeSpec.of(NumberType.HALF_FLOAT, "-65520", "[half_float] supports only finite values"), + OutOfRangeSpec.of(NumberType.FLOAT, "-3.4028235E39", "[float] supports only finite values"), + OutOfRangeSpec.of(NumberType.DOUBLE, "-1.7976931348623157E309", "[double] supports only finite values"), + OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, Float.NaN, "[float] supports only finite values"), OutOfRangeSpec.of(NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"), OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"), - OutOfRangeSpec.of(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values") + OutOfRangeSpec.of(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values"), + + OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.NEGATIVE_INFINITY, "[half_float] supports only finite values"), + OutOfRangeSpec.of(NumberType.FLOAT, Float.NEGATIVE_INFINITY, "[float] supports only finite values"), + OutOfRangeSpec.of(NumberType.DOUBLE, Double.NEGATIVE_INFINITY, "[double] supports only finite values") ); for(OutOfRangeSpec item: inputs) { 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 66db603077c52..13e5e35df685c 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -389,21 +389,29 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier valueSu public void testParseOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( - OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), + OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65520", "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), OutOfRangeSpec.of(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), - OutOfRangeSpec.of(NumberType.HALF_FLOAT, 65504.1, "[half_float] supports only finite values"), - OutOfRangeSpec.of(NumberType.FLOAT, 3.4028235E39, "[float] supports only finite values"), + OutOfRangeSpec.of(NumberType.HALF_FLOAT, 65520f, "[half_float] supports only finite values"), + OutOfRangeSpec.of(NumberType.FLOAT, 3.4028235E39d, "[float] supports only finite values"), OutOfRangeSpec.of(NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"), + OutOfRangeSpec.of(NumberType.HALF_FLOAT, -65520f, "[half_float] supports only finite values"), + OutOfRangeSpec.of(NumberType.FLOAT, -3.4028235E39d, "[float] supports only finite values"), + OutOfRangeSpec.of(NumberType.DOUBLE, new BigDecimal("-1.7976931348623157E309"), "[double] supports only finite values"), + OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, Float.NaN, "[float] supports only finite values"), OutOfRangeSpec.of(NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"), OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"), - OutOfRangeSpec.of(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values") + OutOfRangeSpec.of(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values"), + + OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.NEGATIVE_INFINITY, "[half_float] supports only finite values"), + OutOfRangeSpec.of(NumberType.FLOAT, Float.NEGATIVE_INFINITY, "[float] supports only finite values"), + OutOfRangeSpec.of(NumberType.DOUBLE, Double.NEGATIVE_INFINITY, "[double] supports only finite values") ); for (OutOfRangeSpec item: inputs) {