From 35861574674581e07ec7554b29a02be4bd89b97d Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 20 Jan 2014 13:14:03 +0100 Subject: [PATCH] Make StringFieldMapper.toXContent aware of defaults for not_analyzed fields. StringFieldMapper.toXContent uses the defaults for analyzed fields in order to know which options to add to the builder. This means that if the field is not analyzed and has norms enabled, it will omit to emit `norms.enabled: true`. Parsing the mapping again will result in a StringFieldMapper that has norms disabled. The same fix applies to index options. Close #4760 --- .../index/mapper/core/StringFieldMapper.java | 18 +++-- .../string/SimpleStringMappingTests.java | 73 ++++++++++++++----- 2 files changed, 65 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/mapper/core/StringFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/StringFieldMapper.java index a5b86d0149d..7e603f348fc 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/StringFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/StringFieldMapper.java @@ -126,7 +126,12 @@ public class StringFieldMapper extends AbstractFieldMapper implements Al // if the field is not analyzed, then by default, we should omit norms and have docs only // index options, as probably what the user really wants // if they are set explicitly, we will use those values + // we also change the values on the default field type so that toXContent emits what + // differs from the defaults + FieldType defaultFieldType = new FieldType(Defaults.FIELD_TYPE); if (fieldType.indexed() && !fieldType.tokenized()) { + defaultFieldType.setOmitNorms(true); + defaultFieldType.setIndexOptions(IndexOptions.DOCS_ONLY); if (!omitNormsSet && boost == Defaults.BOOST) { fieldType.setOmitNorms(true); } @@ -134,8 +139,9 @@ public class StringFieldMapper extends AbstractFieldMapper implements Al fieldType.setIndexOptions(IndexOptions.DOCS_ONLY); } } + defaultFieldType.freeze(); StringFieldMapper fieldMapper = new StringFieldMapper(buildNames(context), - boost, fieldType, docValues, nullValue, indexAnalyzer, searchAnalyzer, searchQuotedAnalyzer, + boost, fieldType, defaultFieldType, docValues, nullValue, indexAnalyzer, searchAnalyzer, searchQuotedAnalyzer, positionOffsetGap, ignoreAbove, postingsProvider, docValuesProvider, similarity, normsLoading, fieldDataSettings, context.indexSettings(), multiFieldsBuilder.build(this, context)); fieldMapper.includeInAll(includeInAll); @@ -183,16 +189,13 @@ public class StringFieldMapper extends AbstractFieldMapper implements Al } private String nullValue; - private Boolean includeInAll; - private int positionOffsetGap; - private NamedAnalyzer searchQuotedAnalyzer; - private int ignoreAbove; + private final FieldType defaultFieldType; - protected StringFieldMapper(Names names, float boost, FieldType fieldType, Boolean docValues, + protected StringFieldMapper(Names names, float boost, FieldType fieldType,FieldType defaultFieldType, Boolean docValues, String nullValue, NamedAnalyzer indexAnalyzer, NamedAnalyzer searchAnalyzer, NamedAnalyzer searchQuotedAnalyzer, int positionOffsetGap, int ignoreAbove, PostingsFormatProvider postingsFormat, DocValuesFormatProvider docValuesFormat, @@ -203,6 +206,7 @@ public class StringFieldMapper extends AbstractFieldMapper implements Al if (fieldType.tokenized() && fieldType.indexed() && hasDocValues()) { throw new MapperParsingException("Field [" + names.fullName() + "] cannot be analyzed and have doc values"); } + this.defaultFieldType = defaultFieldType; this.nullValue = nullValue; this.positionOffsetGap = positionOffsetGap; this.searchQuotedAnalyzer = searchQuotedAnalyzer != null ? searchQuotedAnalyzer : this.searchAnalyzer; @@ -211,7 +215,7 @@ public class StringFieldMapper extends AbstractFieldMapper implements Al @Override public FieldType defaultFieldType() { - return Defaults.FIELD_TYPE; + return defaultFieldType; } @Override diff --git a/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java b/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java index f1df0ea58ae..b10ef46e225 100644 --- a/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java @@ -19,11 +19,14 @@ package org.elasticsearch.index.mapper.string; +import com.google.common.collect.ImmutableMap; import org.apache.lucene.index.FieldInfo; import org.apache.lucene.index.FieldInfo.DocValuesType; import org.apache.lucene.index.IndexableField; +import org.apache.lucene.index.IndexableFieldType; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.fielddata.FieldDataType; import org.elasticsearch.index.mapper.*; @@ -74,6 +77,35 @@ public class SimpleStringMappingTests extends ElasticsearchTestCase { assertThat(doc.rootDoc().getField("field"), nullValue()); } + private void assertDefaultAnalyzedFieldType(IndexableFieldType fieldType) { + assertThat(fieldType.omitNorms(), equalTo(false)); + assertThat(fieldType.indexOptions(), equalTo(FieldInfo.IndexOptions.DOCS_AND_FREQS_AND_POSITIONS)); + assertThat(fieldType.storeTermVectors(), equalTo(false)); + assertThat(fieldType.storeTermVectorOffsets(), equalTo(false)); + assertThat(fieldType.storeTermVectorPositions(), equalTo(false)); + assertThat(fieldType.storeTermVectorPayloads(), equalTo(false)); + } + + private void assertEquals(IndexableFieldType ft1, IndexableFieldType ft2) { + assertEquals(ft1.indexed(), ft2.indexed()); + assertEquals(ft1.tokenized(), ft2.tokenized()); + assertEquals(ft1.omitNorms(), ft2.omitNorms()); + assertEquals(ft1.indexOptions(), ft2.indexOptions()); + assertEquals(ft1.storeTermVectors(), ft2.storeTermVectors()); + assertEquals(ft1.docValueType(), ft2.docValueType()); + } + + private void assertParseIdemPotent(IndexableFieldType expected, DocumentMapper mapper) throws Exception { + String mapping = mapper.toXContent(XContentFactory.jsonBuilder().startObject(), new ToXContent.MapParams(ImmutableMap.of())).endObject().string(); + mapper = MapperTestUtils.newParser().parse(mapping); + ParsedDocument doc = mapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .field("field", "2345") + .endObject() + .bytes()); + assertEquals(expected, doc.rootDoc().getField("field").fieldType()); + } + @Test public void testDefaultsForAnalyzed() throws Exception { String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") @@ -88,12 +120,9 @@ public class SimpleStringMappingTests extends ElasticsearchTestCase { .endObject() .bytes()); - assertThat(doc.rootDoc().getField("field").fieldType().omitNorms(), equalTo(false)); - assertThat(doc.rootDoc().getField("field").fieldType().indexOptions(), equalTo(FieldInfo.IndexOptions.DOCS_AND_FREQS_AND_POSITIONS)); - assertThat(doc.rootDoc().getField("field").fieldType().storeTermVectors(), equalTo(false)); - assertThat(doc.rootDoc().getField("field").fieldType().storeTermVectorOffsets(), equalTo(false)); - assertThat(doc.rootDoc().getField("field").fieldType().storeTermVectorPositions(), equalTo(false)); - assertThat(doc.rootDoc().getField("field").fieldType().storeTermVectorPayloads(), equalTo(false)); + IndexableFieldType fieldType = doc.rootDoc().getField("field").fieldType(); + assertDefaultAnalyzedFieldType(fieldType); + assertParseIdemPotent(fieldType, defaultMapper); } @Test @@ -110,12 +139,14 @@ public class SimpleStringMappingTests extends ElasticsearchTestCase { .endObject() .bytes()); - assertThat(doc.rootDoc().getField("field").fieldType().omitNorms(), equalTo(true)); - assertThat(doc.rootDoc().getField("field").fieldType().indexOptions(), equalTo(FieldInfo.IndexOptions.DOCS_ONLY)); - assertThat(doc.rootDoc().getField("field").fieldType().storeTermVectors(), equalTo(false)); - assertThat(doc.rootDoc().getField("field").fieldType().storeTermVectorOffsets(), equalTo(false)); - assertThat(doc.rootDoc().getField("field").fieldType().storeTermVectorPositions(), equalTo(false)); - assertThat(doc.rootDoc().getField("field").fieldType().storeTermVectorPayloads(), equalTo(false)); + IndexableFieldType fieldType = doc.rootDoc().getField("field").fieldType(); + assertThat(fieldType.omitNorms(), equalTo(true)); + assertThat(fieldType.indexOptions(), equalTo(FieldInfo.IndexOptions.DOCS_ONLY)); + assertThat(fieldType.storeTermVectors(), equalTo(false)); + assertThat(fieldType.storeTermVectorOffsets(), equalTo(false)); + assertThat(fieldType.storeTermVectorPositions(), equalTo(false)); + assertThat(fieldType.storeTermVectorPayloads(), equalTo(false)); + assertParseIdemPotent(fieldType, defaultMapper); // now test it explicitly set @@ -131,12 +162,14 @@ public class SimpleStringMappingTests extends ElasticsearchTestCase { .endObject() .bytes()); - assertThat(doc.rootDoc().getField("field").fieldType().omitNorms(), equalTo(false)); - assertThat(doc.rootDoc().getField("field").fieldType().indexOptions(), equalTo(FieldInfo.IndexOptions.DOCS_AND_FREQS)); - assertThat(doc.rootDoc().getField("field").fieldType().storeTermVectors(), equalTo(false)); - assertThat(doc.rootDoc().getField("field").fieldType().storeTermVectorOffsets(), equalTo(false)); - assertThat(doc.rootDoc().getField("field").fieldType().storeTermVectorPositions(), equalTo(false)); - assertThat(doc.rootDoc().getField("field").fieldType().storeTermVectorPayloads(), equalTo(false)); + fieldType = doc.rootDoc().getField("field").fieldType(); + assertThat(fieldType.omitNorms(), equalTo(false)); + assertThat(fieldType.indexOptions(), equalTo(FieldInfo.IndexOptions.DOCS_AND_FREQS)); + assertThat(fieldType.storeTermVectors(), equalTo(false)); + assertThat(fieldType.storeTermVectorOffsets(), equalTo(false)); + assertThat(fieldType.storeTermVectorPositions(), equalTo(false)); + assertThat(fieldType.storeTermVectorPayloads(), equalTo(false)); + assertParseIdemPotent(fieldType, defaultMapper); // also test the deprecated omit_norms @@ -152,7 +185,9 @@ public class SimpleStringMappingTests extends ElasticsearchTestCase { .endObject() .bytes()); - assertThat(doc.rootDoc().getField("field").fieldType().omitNorms(), equalTo(false)); + fieldType = doc.rootDoc().getField("field").fieldType(); + assertThat(fieldType.omitNorms(), equalTo(false)); + assertParseIdemPotent(fieldType, defaultMapper); } @Test