瀏覽代碼

Remove the ability to enable doc values with the `fielddata.format` setting.

Doc values can now only be enabled by setting `doc_values: true` in the
mappings. Removing this feature also means that we can now fail mapping updates
that try to disable doc values.
Adrien Grand 9 年之前
父節點
當前提交
2aaa5e6448

+ 1 - 1
core/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java

@@ -246,7 +246,7 @@ public abstract class FieldMapper extends Mapper implements Cloneable {
             boolean defaultDocValues = defaultDocValues(context.indexCreatedVersion());
             defaultFieldType.setHasDocValues(defaultDocValues);
             if (docValuesSet == false) {
-                fieldType.setHasDocValues(defaultDocValues || fieldDataDocValues);
+                fieldType.setHasDocValues(defaultDocValues);
             }
         }
     }

+ 2 - 4
core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java

@@ -199,10 +199,8 @@ public abstract class MappedFieldType extends FieldType {
         if (stored() != other.stored()) {
             conflicts.add("mapper [" + name() + "] has different [store] values");
         }
-        if (hasDocValues() == false && other.hasDocValues()) {
-            // don't add conflict if this mapper has doc values while the mapper to merge doesn't since doc values are implicitly set
-            // when the doc_values field data format is configured
-            conflicts.add("mapper [" + name() + "] has different [doc_values] values, cannot change from disabled to enabled");
+        if (hasDocValues() != other.hasDocValues()) {
+            conflicts.add("mapper [" + name() + "] has different [doc_values] values");
         }
         if (omitNorms() && !other.omitNorms()) {
             conflicts.add("mapper [" + name() + "] has different [omit_norms] values, cannot change from disable to enabled");

+ 1 - 1
core/src/test/java/org/elasticsearch/index/fielddata/BinaryDVFieldDataTests.java

@@ -46,7 +46,7 @@ public class BinaryDVFieldDataTests extends AbstractFieldDataTestCase {
                 .startObject("properties")
                 .startObject("field")
                 .field("type", "binary")
-                .startObject("fielddata").field("format", "doc_values").endObject()
+                .field("doc_values", true)
                 .endObject()
                 .endObject()
                 .endObject().endObject().string();

+ 17 - 29
core/src/test/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java

@@ -38,13 +38,10 @@ public abstract class FieldTypeTestCase extends ESTestCase {
         public final String property;
         /** true if this modifier only makes types incompatible in strict mode, false otherwise */
         public final boolean strictOnly;
-        /** true if reversing the order of checkCompatibility arguments should result in the same conflicts, false otherwise **/
-        public final boolean symmetric;
 
-        public Modifier(String property, boolean strictOnly, boolean symmetric) {
+        public Modifier(String property, boolean strictOnly) {
             this.property = property;
             this.strictOnly = strictOnly;
-            this.symmetric = symmetric;
         }
 
         /** Modifies the property */
@@ -57,25 +54,25 @@ public abstract class FieldTypeTestCase extends ESTestCase {
     }
 
     private final List<Modifier> modifiers = new ArrayList<>(Arrays.asList(
-        new Modifier("boost", true, true) {
+        new Modifier("boost", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setBoost(1.1f);
             }
         },
-        new Modifier("doc_values", false, false) {
+        new Modifier("doc_values", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setHasDocValues(ft.hasDocValues() == false);
             }
         },
-        new Modifier("analyzer", false, true) {
+        new Modifier("analyzer", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setIndexAnalyzer(new NamedAnalyzer("bar", new StandardAnalyzer()));
             }
         },
-        new Modifier("analyzer", false, true) {
+        new Modifier("analyzer", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setIndexAnalyzer(new NamedAnalyzer("bar", new StandardAnalyzer()));
@@ -85,13 +82,13 @@ public abstract class FieldTypeTestCase extends ESTestCase {
                 other.setIndexAnalyzer(new NamedAnalyzer("foo", new StandardAnalyzer()));
             }
         },
-        new Modifier("search_analyzer", true, true) {
+        new Modifier("search_analyzer", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setSearchAnalyzer(new NamedAnalyzer("bar", new StandardAnalyzer()));
             }
         },
-        new Modifier("search_analyzer", true, true) {
+        new Modifier("search_analyzer", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setSearchAnalyzer(new NamedAnalyzer("bar", new StandardAnalyzer()));
@@ -101,13 +98,13 @@ public abstract class FieldTypeTestCase extends ESTestCase {
                 other.setSearchAnalyzer(new NamedAnalyzer("foo", new StandardAnalyzer()));
             }
         },
-        new Modifier("search_quote_analyzer", true, true) {
+        new Modifier("search_quote_analyzer", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setSearchQuoteAnalyzer(new NamedAnalyzer("bar", new StandardAnalyzer()));
             }
         },
-        new Modifier("search_quote_analyzer", true, true) {
+        new Modifier("search_quote_analyzer", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setSearchQuoteAnalyzer(new NamedAnalyzer("bar", new StandardAnalyzer()));
@@ -117,13 +114,13 @@ public abstract class FieldTypeTestCase extends ESTestCase {
                 other.setSearchQuoteAnalyzer(new NamedAnalyzer("foo", new StandardAnalyzer()));
             }
         },
-        new Modifier("similarity", false, true) {
+        new Modifier("similarity", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setSimilarity(new BM25SimilarityProvider("foo", Settings.EMPTY));
             }
         },
-        new Modifier("similarity", false, true) {
+        new Modifier("similarity", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setSimilarity(new BM25SimilarityProvider("foo", Settings.EMPTY));
@@ -133,19 +130,19 @@ public abstract class FieldTypeTestCase extends ESTestCase {
                 other.setSimilarity(new BM25SimilarityProvider("bar", Settings.EMPTY));
             }
         },
-        new Modifier("norms.loading", true, true) {
+        new Modifier("norms.loading", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setNormsLoading(MappedFieldType.Loading.LAZY);
             }
         },
-        new Modifier("fielddata", true, true) {
+        new Modifier("fielddata", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setFieldDataType(new FieldDataType("foo", Settings.builder().put("loading", "eager").build()));
             }
         },
-        new Modifier("null_value", true, true) {
+        new Modifier("null_value", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ft.setNullValue(dummyNullValue);
@@ -334,23 +331,14 @@ public abstract class FieldTypeTestCase extends ESTestCase {
                 assertCompatible(modifier.property, ft1, ft2, false);
                 assertNotCompatible(modifier.property, ft1, ft2, true, conflicts);
                 assertCompatible(modifier.property, ft2, ft1, false); // always symmetric when not strict
-                if (modifier.symmetric) {
-                    assertNotCompatible(modifier.property, ft2, ft1, true, conflicts);
-                } else {
-                    assertCompatible(modifier.property, ft2, ft1, true);
-                }
+                assertNotCompatible(modifier.property, ft2, ft1, true, conflicts);
             } else {
                 // not compatible whether strict or not
                 String conflict = "different [" + modifier.property + "]";
                 assertNotCompatible(modifier.property, ft1, ft2, true, conflict);
                 assertNotCompatible(modifier.property, ft1, ft2, false, conflict);
-                if (modifier.symmetric) {
-                    assertNotCompatible(modifier.property, ft2, ft1, true, conflict);
-                    assertNotCompatible(modifier.property, ft2, ft1, false, conflict);
-                } else {
-                    assertCompatible(modifier.property, ft2, ft1, true);
-                    assertCompatible(modifier.property, ft2, ft1, false);
-                }
+                assertNotCompatible(modifier.property, ft2, ft1, true, conflict);
+                assertNotCompatible(modifier.property, ft2, ft1, false, conflict);
             }
         }
     }

+ 3 - 3
core/src/test/java/org/elasticsearch/index/mapper/core/CompletionFieldTypeTests.java

@@ -34,21 +34,21 @@ public class CompletionFieldTypeTests extends FieldTypeTestCase {
 
     @Before
     public void setupProperties() {
-        addModifier(new Modifier("preserve_separators", false, true) {
+        addModifier(new Modifier("preserve_separators", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 CompletionFieldMapper.CompletionFieldType cft = (CompletionFieldMapper.CompletionFieldType)ft;
                 cft.setPreserveSep(false);
             }
         });
-        addModifier(new Modifier("preserve_position_increments", false, true) {
+        addModifier(new Modifier("preserve_position_increments", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 CompletionFieldMapper.CompletionFieldType cft = (CompletionFieldMapper.CompletionFieldType)ft;
                 cft.setPreservePositionIncrements(false);
             }
         });
-        addModifier(new Modifier("context_mappings", false, true) {
+        addModifier(new Modifier("context_mappings", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 CompletionFieldMapper.CompletionFieldType cft = (CompletionFieldMapper.CompletionFieldType)ft;

+ 3 - 3
core/src/test/java/org/elasticsearch/index/mapper/core/DateFieldTypeTests.java

@@ -35,19 +35,19 @@ public class DateFieldTypeTests extends FieldTypeTestCase {
     @Before
     public void setupProperties() {
         setDummyNullValue(10);
-        addModifier(new Modifier("format", true, true) {
+        addModifier(new Modifier("format", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((DateFieldMapper.DateFieldType) ft).setDateTimeFormatter(Joda.forPattern("basic_week_date", Locale.ROOT));
             }
         });
-        addModifier(new Modifier("locale", true, true) {
+        addModifier(new Modifier("locale", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((DateFieldMapper.DateFieldType) ft).setDateTimeFormatter(Joda.forPattern("date_optional_time", Locale.CANADA));
             }
         });
-        addModifier(new Modifier("numeric_resolution", true, true) {
+        addModifier(new Modifier("numeric_resolution", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((DateFieldMapper.DateFieldType)ft).setTimeUnit(TimeUnit.HOURS);

+ 2 - 2
core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldTypeTests.java

@@ -32,13 +32,13 @@ public class GeoPointFieldTypeTests extends FieldTypeTestCase {
 
     @Before
     public void setupProperties() {
-        addModifier(new Modifier("geohash", false, true) {
+        addModifier(new Modifier("geohash", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((BaseGeoPointFieldMapper.GeoPointFieldType)ft).setGeoHashEnabled(new StringFieldMapper.StringFieldType(), 1, true);
             }
         });
-        addModifier(new Modifier("lat_lon", false, true) {
+        addModifier(new Modifier("lat_lon", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((BaseGeoPointFieldMapper.GeoPointFieldType)ft).setLatLonEnabled(new DoubleFieldMapper.DoubleFieldType(), new DoubleFieldMapper.DoubleFieldType());

+ 6 - 6
core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldTypeTests.java

@@ -31,37 +31,37 @@ public class GeoShapeFieldTypeTests extends FieldTypeTestCase {
 
     @Before
     public void setupProperties() {
-        addModifier(new Modifier("tree", false, true) {
+        addModifier(new Modifier("tree", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((GeoShapeFieldMapper.GeoShapeFieldType)ft).setTree("quadtree");
             }
         });
-        addModifier(new Modifier("strategy", false, true) {
+        addModifier(new Modifier("strategy", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((GeoShapeFieldMapper.GeoShapeFieldType)ft).setStrategyName("term");
             }
         });
-        addModifier(new Modifier("tree_levels", false, true) {
+        addModifier(new Modifier("tree_levels", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((GeoShapeFieldMapper.GeoShapeFieldType)ft).setTreeLevels(10);
             }
         });
-        addModifier(new Modifier("precision", false, true) {
+        addModifier(new Modifier("precision", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((GeoShapeFieldMapper.GeoShapeFieldType)ft).setPrecisionInMeters(20);
             }
         });
-        addModifier(new Modifier("distance_error_pct", true, true) {
+        addModifier(new Modifier("distance_error_pct", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((GeoShapeFieldMapper.GeoShapeFieldType)ft).setDefaultDistanceErrorPct(0.5);
             }
         });
-        addModifier(new Modifier("orientation", true, true) {
+        addModifier(new Modifier("orientation", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((GeoShapeFieldMapper.GeoShapeFieldType)ft).setOrientation(ShapeBuilder.Orientation.LEFT);

+ 1 - 1
core/src/test/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldTypeTests.java

@@ -30,7 +30,7 @@ public class FieldNamesFieldTypeTests extends FieldTypeTestCase {
 
     @Before
     public void setupProperties() {
-        addModifier(new Modifier("enabled", true, true) {
+        addModifier(new Modifier("enabled", true) {
             @Override
             public void modify(MappedFieldType ft) {
                 FieldNamesFieldMapper.FieldNamesFieldType fnft = (FieldNamesFieldMapper.FieldNamesFieldType)ft;

+ 0 - 42
core/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java

@@ -33,7 +33,6 @@ import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.index.IndexService;
-import org.elasticsearch.index.fielddata.FieldDataType;
 import org.elasticsearch.index.mapper.ContentPath;
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.DocumentMapperParser;
@@ -64,7 +63,6 @@ import static org.hamcrest.Matchers.nullValue;
 /**
  */
 public class SimpleStringMappingTests extends ESSingleNodeTestCase {
-    private static Settings DOC_VALUES_SETTINGS = Settings.builder().put(FieldDataType.FORMAT_KEY, FieldDataType.DOC_VALUES_FORMAT_VALUE).build();
 
     @Override
     protected Collection<Class<? extends Plugin>> getPlugins() {
@@ -371,46 +369,6 @@ public class SimpleStringMappingTests extends ESSingleNodeTestCase {
         assertThat(doc.rootDoc().getField("field6").fieldType().storeTermVectorPayloads(), equalTo(true));
     }
 
-    public void testDocValuesFielddata() throws Exception {
-        IndexService indexService = createIndex("index");
-        DocumentMapperParser parser = indexService.mapperService().documentMapperParser();
-        final BuilderContext ctx = new BuilderContext(indexService.getIndexSettings().getSettings(), new ContentPath(1));
-
-        assertFalse(new Builder("anything").index(false).build(ctx).fieldType().hasDocValues());
-        assertTrue(new Builder("anything").index(false).fieldDataSettings(DOC_VALUES_SETTINGS).build(ctx).fieldType().hasDocValues());
-        assertTrue(new Builder("anything").index(false).docValues(true).build(ctx).fieldType().hasDocValues());
-
-        String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
-            .startObject("properties")
-            .startObject("str1")
-                .field("type", "string")
-                .startObject("fielddata")
-                    .field("format", "paged_bytes")
-                .endObject()
-            .endObject()
-            .startObject("str2")
-                .field("type", "string")
-                .field("index", "not_analyzed")
-                .startObject("fielddata")
-                    .field("format", "doc_values")
-                .endObject()
-            .endObject()
-            .endObject()
-            .endObject().endObject().string();
-
-        DocumentMapper defaultMapper = parser.parse("type", new CompressedXContent(mapping));
-
-        ParsedDocument parsedDoc = defaultMapper.parse("test", "type", "1", XContentFactory.jsonBuilder()
-            .startObject()
-            .field("str1", "1234")
-            .field("str2", "1234")
-            .endObject()
-            .bytes());
-        final Document doc = parsedDoc.rootDoc();
-        assertEquals(DocValuesType.NONE, docValuesType(doc, "str1"));
-        assertEquals(DocValuesType.SORTED_SET, docValuesType(doc, "str2"));
-    }
-
     public void testDocValues() throws Exception {
         // doc values only work on non-analyzed content
         final BuilderContext ctx = new BuilderContext(indexService.getIndexSettings().getSettings(), new ContentPath(1));

+ 6 - 0
docs/reference/migration/migrate_3_0.asciidoc

@@ -293,6 +293,12 @@ values as follows:
 }
 ----
 
+==== `fielddata.format`
+
+Setting `fielddata.format: doc_values` in the mappings used to implicitly
+enable doc values on a field. This no longer works: the only way to enable or
+disable doc values is by using the `doc_values` property of mappings.
+
 
 [[breaking_30_plugins]]
 === Plugin changes

+ 1 - 3
modules/lang-groovy/src/test/java/org/elasticsearch/messy/tests/EquivalenceTests.java

@@ -194,9 +194,7 @@ public class EquivalenceTests extends ESIntegTestCase {
                             .startObject("doc_values")
                                 .field("type", "string")
                                 .field("index", "no")
-                                .startObject("fielddata")
-                                    .field("format", "doc_values")
-                                .endObject()
+                                .field("doc_values", true)
                             .endObject()
                         .endObject()
                         .endObject()