Browse Source

Fix and test handling of `null_value`. #18090

This was mostly untested and had some bugs.

Closes #18085
Adrien Grand 9 years ago
parent
commit
b91df36a62

+ 7 - 3
core/src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java

@@ -36,7 +36,6 @@ import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.joda.DateMathParser;
 import org.elasticsearch.common.joda.FormatDateTimeFormatter;
 import org.elasticsearch.common.joda.Joda;
-import org.elasticsearch.common.network.InetAddresses;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.Fuzziness;
 import org.elasticsearch.common.util.LocaleUtils;
@@ -152,7 +151,7 @@ public class DateFieldMapper extends FieldMapper implements AllFieldMapper.Inclu
                     if (propNode == null) {
                         throw new MapperParsingException("Property [null_value] cannot be null.");
                     }
-                    builder.nullValue(InetAddresses.forString(propNode.toString()));
+                    builder.nullValue(propNode.toString());
                     iterator.remove();
                 } else if (propName.equals("ignore_malformed")) {
                     builder.ignoreMalformed(TypeParsers.nodeBooleanValue("ignore_malformed", propNode, parserContext));
@@ -561,7 +560,7 @@ public class DateFieldMapper extends FieldMapper implements AllFieldMapper.Inclu
                 dateAsString = dateAsObject.toString();
             }
         } else {
-            dateAsString = context.parser().text();
+            dateAsString = context.parser().textOrNull();
         }
 
         if (dateAsString == null) {
@@ -615,6 +614,11 @@ public class DateFieldMapper extends FieldMapper implements AllFieldMapper.Inclu
         if (includeDefaults || ignoreMalformed.explicit()) {
             builder.field("ignore_malformed", ignoreMalformed.value());
         }
+
+        if (includeDefaults || fieldType().nullValue() != null) {
+            builder.field("null_value", fieldType().nullValueAsString());
+        }
+
         if (includeInAll != null) {
             builder.field("include_in_all", includeInAll);
         } else if (includeDefaults) {

+ 35 - 2
core/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java

@@ -366,8 +366,15 @@ public class NumberFieldMapper extends FieldMapper implements AllFieldMapper.Inc
         BYTE("byte", NumericType.BYTE) {
             @Override
             Byte parse(Object value) {
-                if (value instanceof Byte) {
-                    return (Byte) value;
+                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 (doubleValue % 1 != 0) {
+                        throw new IllegalArgumentException("Value [" + value + "] has a decimal part");
+                    }
+                    return ((Number) value).byteValue();
                 }
                 if (value instanceof BytesRef) {
                     value = ((BytesRef) value).utf8ToString();
@@ -426,6 +433,13 @@ public class NumberFieldMapper extends FieldMapper implements AllFieldMapper.Inc
             @Override
             Short parse(Object value) {
                 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 (doubleValue % 1 != 0) {
+                        throw new IllegalArgumentException("Value [" + value + "] has a decimal part");
+                    }
                     return ((Number) value).shortValue();
                 }
                 if (value instanceof BytesRef) {
@@ -485,6 +499,13 @@ public class NumberFieldMapper extends FieldMapper implements AllFieldMapper.Inc
             @Override
             Integer parse(Object value) {
                 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 (doubleValue % 1 != 0) {
+                        throw new IllegalArgumentException("Value [" + value + "] has a decimal part");
+                    }
                     return ((Number) value).intValue();
                 }
                 if (value instanceof BytesRef) {
@@ -581,6 +602,13 @@ public class NumberFieldMapper extends FieldMapper implements AllFieldMapper.Inc
             @Override
             Long parse(Object value) {
                 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 (doubleValue % 1 != 0) {
+                        throw new IllegalArgumentException("Value [" + value + "] has a decimal part");
+                    }
                     return ((Number) value).longValue();
                 }
                 if (value instanceof BytesRef) {
@@ -944,6 +972,11 @@ public class NumberFieldMapper extends FieldMapper implements AllFieldMapper.Inc
         if (includeDefaults || coerce.explicit()) {
             builder.field("coerce", coerce.value());
         }
+
+        if (includeDefaults || fieldType().nullValue() != null) {
+            builder.field("null_value", fieldType().nullValue());
+        }
+
         if (includeInAll != null) {
             builder.field("include_in_all", includeInAll);
         } else if (includeDefaults) {

+ 5 - 1
core/src/main/java/org/elasticsearch/index/mapper/ip/IpFieldMapper.java

@@ -339,7 +339,7 @@ public class IpFieldMapper extends FieldMapper implements AllFieldMapper.Include
         if (context.externalValueSet()) {
             addressAsObject = context.externalValue();
         } else {
-            addressAsObject = context.parser().text();
+            addressAsObject = context.parser().textOrNull();
         }
 
         if (addressAsObject == null) {
@@ -395,6 +395,10 @@ public class IpFieldMapper extends FieldMapper implements AllFieldMapper.Include
     protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
         super.doXContentBody(builder, includeDefaults, params);
 
+        if (includeDefaults || fieldType().nullValue() != null) {
+            builder.field("null_value", InetAddresses.toAddrString((InetAddress) fieldType().nullValue()));
+        }
+
         if (includeDefaults || ignoreMalformed.explicit()) {
             builder.field("ignore_malformed", ignoreMalformed.value());
         }

+ 51 - 0
core/src/test/java/org/elasticsearch/index/mapper/core/DateFieldMapperTests.java

@@ -251,4 +251,55 @@ public class DateFieldMapperTests extends ESSingleNodeTestCase {
                 .endObject()
                 .bytes());
     }
+
+    public void testNullValue() throws IOException {
+        String mapping = XContentFactory.jsonBuilder().startObject()
+                .startObject("type")
+                    .startObject("properties")
+                        .startObject("field")
+                            .field("type", "date")
+                        .endObject()
+                    .endObject()
+                .endObject().endObject().string();
+
+        DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping));
+        assertEquals(mapping, mapper.mappingSource().toString());
+
+        ParsedDocument doc = mapper.parse("test", "type", "1", XContentFactory.jsonBuilder()
+                .startObject()
+                .nullField("field")
+                .endObject()
+                .bytes());
+        assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field"));
+
+        mapping = XContentFactory.jsonBuilder().startObject()
+                .startObject("type")
+                    .startObject("properties")
+                        .startObject("field")
+                            .field("type", "date")
+                            .field("null_value", "2016-03-11")
+                        .endObject()
+                    .endObject()
+                .endObject().endObject().string();
+
+        mapper = parser.parse("type", new CompressedXContent(mapping));
+        assertEquals(mapping, mapper.mappingSource().toString());
+
+        doc = mapper.parse("test", "type", "1", XContentFactory.jsonBuilder()
+                .startObject()
+                .nullField("field")
+                .endObject()
+                .bytes());
+        IndexableField[] fields = doc.rootDoc().getFields("field");
+        assertEquals(2, fields.length);
+        IndexableField pointField = fields[0];
+        assertEquals(1, pointField.fieldType().pointDimensionCount());
+        assertEquals(8, pointField.fieldType().pointNumBytes());
+        assertFalse(pointField.fieldType().stored());
+        assertEquals(1457654400000L, pointField.numericValue().longValue());
+        IndexableField dvField = fields[1];
+        assertEquals(DocValuesType.SORTED_NUMERIC, dvField.fieldType().docValuesType());
+        assertEquals(1457654400000L, dvField.numericValue().longValue());
+        assertFalse(dvField.fieldType().stored());
+    }
 }

+ 16 - 2
core/src/test/java/org/elasticsearch/index/mapper/core/KeywordFieldMapperTests.java

@@ -126,14 +126,28 @@ public class KeywordFieldMapperTests extends ESSingleNodeTestCase {
 
     public void testNullValue() throws IOException {
         String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
-                .startObject("properties").startObject("field").field("type", "keyword").field("null_value", "uri").endObject().endObject()
+                .startObject("properties").startObject("field").field("type", "keyword").endObject().endObject()
                 .endObject().endObject().string();
 
         DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping));
-
         assertEquals(mapping, mapper.mappingSource().toString());
 
         ParsedDocument doc = mapper.parse("test", "type", "1", XContentFactory.jsonBuilder()
+                .startObject()
+                .nullField("field")
+                .endObject()
+                .bytes());
+        assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field"));
+
+        mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+                .startObject("properties").startObject("field").field("type", "keyword").field("null_value", "uri").endObject().endObject()
+                .endObject().endObject().string();
+
+        mapper = parser.parse("type", new CompressedXContent(mapping));
+
+        assertEquals(mapping, mapper.mappingSource().toString());
+
+        doc = mapper.parse("test", "type", "1", XContentFactory.jsonBuilder()
                 .startObject()
                 .endObject()
                 .bytes());

+ 61 - 0
core/src/test/java/org/elasticsearch/index/mapper/core/NumberFieldMapperTests.java

@@ -316,4 +316,65 @@ public class NumberFieldMapperTests extends ESSingleNodeTestCase {
             assertThat(e.getMessage(), containsString("Mapping definition for [foo] has unsupported parameters:  [norms"));
         }
     }
+
+    public void testNullValue() throws IOException {
+        for (String type : TYPES) {
+            doTestNullValue(type);
+        }
+    }
+
+    private void doTestNullValue(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("test", "type", "1", XContentFactory.jsonBuilder()
+                .startObject()
+                .nullField("field")
+                .endObject()
+                .bytes());
+        assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field"));
+
+        Object missing;
+        if (Arrays.asList("float", "double").contains(type)) {
+            missing = 123d;
+        } else {
+            missing = 123L;
+        }
+        mapping = XContentFactory.jsonBuilder().startObject()
+                .startObject("type")
+                    .startObject("properties")
+                        .startObject("field")
+                            .field("type", type)
+                            .field("null_value", missing)
+                        .endObject()
+                    .endObject()
+                .endObject().endObject().string();
+
+        mapper = parser.parse("type", new CompressedXContent(mapping));
+        assertEquals(mapping, mapper.mappingSource().toString());
+
+        doc = mapper.parse("test", "type", "1", XContentFactory.jsonBuilder()
+                .startObject()
+                .nullField("field")
+                .endObject()
+                .bytes());
+        IndexableField[] fields = doc.rootDoc().getFields("field");
+        assertEquals(2, fields.length);
+        IndexableField pointField = fields[0];
+        assertEquals(1, pointField.fieldType().pointDimensionCount());
+        assertFalse(pointField.fieldType().stored());
+        assertEquals(123, pointField.numericValue().doubleValue(), 0d);
+        IndexableField dvField = fields[1];
+        assertEquals(DocValuesType.SORTED_NUMERIC, dvField.fieldType().docValuesType());
+        assertFalse(dvField.fieldType().stored());
+    }
 }

+ 31 - 0
core/src/test/java/org/elasticsearch/index/mapper/core/NumberFieldTypeTests.java

@@ -75,4 +75,35 @@ public class NumberFieldTypeTests extends FieldTypeTestCase {
                 () -> ft.rangeQuery("1", "3", true, true));
         assertEquals("Cannot search on field [field] since it is not indexed.", e.getMessage());
     }
+
+    public void testConversions() {
+        assertEquals((byte) 3, NumberType.BYTE.parse(3d));
+        assertEquals((short) 3, NumberType.SHORT.parse(3d));
+        assertEquals(3, NumberType.INTEGER.parse(3d));
+        assertEquals(3L, NumberType.LONG.parse(3d));
+        assertEquals(3f, NumberType.FLOAT.parse(3d));
+        assertEquals(3d, NumberType.DOUBLE.parse(3d));
+
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> NumberType.BYTE.parse(3.5));
+        assertEquals("Value [3.5] has a decimal part", e.getMessage());
+        e = expectThrows(IllegalArgumentException.class, () -> NumberType.SHORT.parse(3.5));
+        assertEquals("Value [3.5] has a decimal part", e.getMessage());
+        e = expectThrows(IllegalArgumentException.class, () -> NumberType.INTEGER.parse(3.5));
+        assertEquals("Value [3.5] has a decimal part", e.getMessage());
+        e = expectThrows(IllegalArgumentException.class, () -> NumberType.LONG.parse(3.5));
+        assertEquals("Value [3.5] has a decimal part", e.getMessage());
+        assertEquals(3.5f, NumberType.FLOAT.parse(3.5));
+        assertEquals(3.5d, NumberType.DOUBLE.parse(3.5));
+
+        e = expectThrows(IllegalArgumentException.class, () -> NumberType.BYTE.parse(128));
+        assertEquals("Value [128] is out of range for a byte", e.getMessage());
+        e = expectThrows(IllegalArgumentException.class, () -> NumberType.SHORT.parse(65536));
+        assertEquals("Value [65536] is out of range for a short", e.getMessage());
+        e = expectThrows(IllegalArgumentException.class, () -> NumberType.INTEGER.parse(2147483648L));
+        assertEquals("Value [2147483648] is out of range for an integer", e.getMessage());
+        e = expectThrows(IllegalArgumentException.class, () -> NumberType.LONG.parse(10000000000000000000d));
+        assertEquals("Value [1.0E19] is out of range for a long", e.getMessage());
+        assertEquals(1.1f, NumberType.FLOAT.parse(1.1)); // accuracy loss is expected
+        assertEquals(1.1d, NumberType.DOUBLE.parse(1.1));
+    }
 }

+ 52 - 0
core/src/test/java/org/elasticsearch/index/mapper/ip/IpFieldMapperTests.java

@@ -36,6 +36,7 @@ import org.junit.Before;
 
 import static org.hamcrest.Matchers.containsString;
 
+import java.io.IOException;
 import java.net.InetAddress;
 
 public class IpFieldMapperTests extends ESSingleNodeTestCase {
@@ -217,4 +218,55 @@ public class IpFieldMapperTests extends ESSingleNodeTestCase {
         fields = doc.rootDoc().getFields("_all");
         assertEquals(0, fields.length);
     }
+
+    public void testNullValue() throws IOException {
+        String mapping = XContentFactory.jsonBuilder().startObject()
+                .startObject("type")
+                    .startObject("properties")
+                        .startObject("field")
+                            .field("type", "ip")
+                        .endObject()
+                    .endObject()
+                .endObject().endObject().string();
+
+        DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping));
+        assertEquals(mapping, mapper.mappingSource().toString());
+
+        ParsedDocument doc = mapper.parse("test", "type", "1", XContentFactory.jsonBuilder()
+                .startObject()
+                .nullField("field")
+                .endObject()
+                .bytes());
+        assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field"));
+
+        mapping = XContentFactory.jsonBuilder().startObject()
+                .startObject("type")
+                    .startObject("properties")
+                        .startObject("field")
+                            .field("type", "ip")
+                            .field("null_value", "::1")
+                        .endObject()
+                    .endObject()
+                .endObject().endObject().string();
+
+        mapper = parser.parse("type", new CompressedXContent(mapping));
+        assertEquals(mapping, mapper.mappingSource().toString());
+
+        doc = mapper.parse("test", "type", "1", XContentFactory.jsonBuilder()
+                .startObject()
+                .nullField("field")
+                .endObject()
+                .bytes());
+        IndexableField[] fields = doc.rootDoc().getFields("field");
+        assertEquals(2, fields.length);
+        IndexableField pointField = fields[0];
+        assertEquals(1, pointField.fieldType().pointDimensionCount());
+        assertEquals(16, pointField.fieldType().pointNumBytes());
+        assertFalse(pointField.fieldType().stored());
+        assertEquals(new BytesRef(InetAddressPoint.encode(InetAddresses.forString("::1"))), pointField.binaryValue());
+        IndexableField dvField = fields[1];
+        assertEquals(DocValuesType.SORTED_SET, dvField.fieldType().docValuesType());
+        assertEquals(new BytesRef(InetAddressPoint.encode(InetAddresses.forString("::1"))), dvField.binaryValue());
+        assertFalse(dvField.fieldType().stored());
+    }
 }