Browse Source

Fix cross type mapping updates for `boolean` fields. #17882

Boolean fields were not handled in `DocumentParser.createBuilderFromFieldType`.
This also improves the logic a bit by falling back to the default mapping of
the given type insteah of hard-coding every case and throws a better exception
than a NPE if no dynamic mappings could be created.

Closes #17879
Adrien Grand 9 years ago
parent
commit
ab168996b6

+ 14 - 38
core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

@@ -22,6 +22,7 @@ package org.elasticsearch.index.mapper;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 
@@ -593,9 +594,6 @@ final class DocumentParser {
         Mapper.Builder builder = null;
         if (fieldType instanceof StringFieldType) {
             builder = context.root().findTemplateBuilder(context, currentFieldName, "string", "string");
-            if (builder == null) {
-                builder = new StringFieldMapper.Builder(currentFieldName);
-            }
         } else if (fieldType instanceof TextFieldType) {
             builder = context.root().findTemplateBuilder(context, currentFieldName, "text", "string");
             if (builder == null) {
@@ -604,45 +602,39 @@ final class DocumentParser {
             }
         } else if (fieldType instanceof KeywordFieldType) {
             builder = context.root().findTemplateBuilder(context, currentFieldName, "keyword", "string");
-            if (builder == null) {
-                builder = new KeywordFieldMapper.Builder(currentFieldName);
-            }
         } else {
             switch (fieldType.typeName()) {
-            case "date":
+            case DateFieldMapper.CONTENT_TYPE:
                 builder = context.root().findTemplateBuilder(context, currentFieldName, "date");
-                if (builder == null) {
-                    builder = newDateBuilder(currentFieldName, null, Version.indexCreated(context.indexSettings()));
-                }
                 break;
             case "long":
                 builder = context.root().findTemplateBuilder(context, currentFieldName, "long");
-                if (builder == null) {
-                    builder = newLongBuilder(currentFieldName, Version.indexCreated(context.indexSettings()));
-                }
                 break;
             case "double":
                 builder = context.root().findTemplateBuilder(context, currentFieldName, "double");
-                if (builder == null) {
-                    builder = newDoubleBuilder(currentFieldName, Version.indexCreated(context.indexSettings()));
-                }
                 break;
             case "integer":
                 builder = context.root().findTemplateBuilder(context, currentFieldName, "integer");
-                if (builder == null) {
-                    builder = newIntBuilder(currentFieldName, Version.indexCreated(context.indexSettings()));
-                }
                 break;
             case "float":
                 builder = context.root().findTemplateBuilder(context, currentFieldName, "float");
-                if (builder == null) {
-                    builder = newFloatBuilder(currentFieldName, Version.indexCreated(context.indexSettings()));
-                }
+                break;
+            case BooleanFieldMapper.CONTENT_TYPE:
+                builder = context.root().findTemplateBuilder(context, currentFieldName, "boolean");
                 break;
             default:
                 break;
             }
         }
+        if (builder == null) {
+            Mapper.TypeParser.ParserContext parserContext = context.docMapperParser().parserContext(currentFieldName);
+            Mapper.TypeParser typeParser = parserContext.typeParser(fieldType.typeName());
+            if (typeParser == null) {
+                throw new MapperParsingException("Cannot generate dynamic mappings of type [" + fieldType.typeName()
+                    + "] for [" + currentFieldName + "]");
+            }
+            builder = typeParser.parse(currentFieldName, new HashMap<>(), parserContext);
+        }
         return builder;
     }
 
@@ -654,22 +646,6 @@ final class DocumentParser {
         }
     }
 
-    private static Mapper.Builder<?, ?> newIntBuilder(String name, Version indexCreated) {
-        if (indexCreated.onOrAfter(Version.V_5_0_0)) {
-            return new NumberFieldMapper.Builder(name, NumberFieldMapper.NumberType.INTEGER);
-        } else {
-            return new LegacyIntegerFieldMapper.Builder(name);
-        }
-    }
-
-    private static Mapper.Builder<?, ?> newDoubleBuilder(String name, Version indexCreated) {
-        if (indexCreated.onOrAfter(Version.V_5_0_0)) {
-            return new NumberFieldMapper.Builder(name, NumberFieldMapper.NumberType.DOUBLE);
-        } else {
-            return new LegacyDoubleFieldMapper.Builder(name);
-        }
-    }
-
     private static Mapper.Builder<?, ?> newFloatBuilder(String name, Version indexCreated) {
         if (indexCreated.onOrAfter(Version.V_5_0_0)) {
             return new NumberFieldMapper.Builder(name, NumberFieldMapper.NumberType.FLOAT);

+ 13 - 1
core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java

@@ -31,6 +31,8 @@ import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.IndexService;
+import org.elasticsearch.index.mapper.core.BooleanFieldMapper;
+import org.elasticsearch.index.mapper.core.BooleanFieldMapper.BooleanFieldType;
 import org.elasticsearch.index.mapper.core.DateFieldMapper;
 import org.elasticsearch.index.mapper.core.DateFieldMapper.DateFieldType;
 import org.elasticsearch.index.mapper.core.NumberFieldMapper;
@@ -429,7 +431,8 @@ public class DynamicMappingTests extends ESSingleNodeTestCase {
                 "my_field3", "type=long,doc_values=false",
                 "my_field4", "type=float,index=false",
                 "my_field5", "type=double,store=true",
-                "my_field6", "type=date,doc_values=false");
+                "my_field6", "type=date,doc_values=false",
+                "my_field7", "type=boolean,doc_values=false");
 
         // Even if the dynamic type of our new field is long, we already have a mapping for the same field
         // of type string so it should be mapped as a string
@@ -442,6 +445,7 @@ public class DynamicMappingTests extends ESSingleNodeTestCase {
                     .field("my_field4", 45)
                     .field("my_field5", 46)
                     .field("my_field6", 47)
+                    .field("my_field7", true)
                 .endObject());
         Mapper myField1Mapper = null;
         Mapper myField2Mapper = null;
@@ -449,6 +453,7 @@ public class DynamicMappingTests extends ESSingleNodeTestCase {
         Mapper myField4Mapper = null;
         Mapper myField5Mapper = null;
         Mapper myField6Mapper = null;
+        Mapper myField7Mapper = null;
         for (Mapper m : update) {
             switch (m.name()) {
             case "my_field1":
@@ -469,6 +474,9 @@ public class DynamicMappingTests extends ESSingleNodeTestCase {
             case "my_field6":
                 myField6Mapper = m;
                 break;
+            case "my_field7":
+                myField7Mapper = m;
+                break;
             }
         }
         assertNotNull(myField1Mapper);
@@ -502,6 +510,10 @@ public class DynamicMappingTests extends ESSingleNodeTestCase {
         assertTrue(myField6Mapper instanceof DateFieldMapper);
         assertFalse(((DateFieldType) ((DateFieldMapper) myField6Mapper).fieldType()).hasDocValues());
 
+        assertNotNull(myField7Mapper);
+        assertTrue(myField7Mapper instanceof BooleanFieldMapper);
+        assertFalse(((BooleanFieldType) ((BooleanFieldMapper) myField7Mapper).fieldType()).hasDocValues());
+
         // This can't work
         try {
             parse(newMapper, indexService.mapperService().documentMapperParser(),