Skip to content

Commit

Permalink
Make StringFieldMapper.toXContent aware of defaults for not_analyzed …
Browse files Browse the repository at this point in the history 
…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 elastic#4760
  • Loading branch information
@jpountz
jpountz committed Jan 20, 2014
1 parent 3a55897 commit 763cf2c
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 26 deletions.
18 changes: 11 additions & 7 deletions src/main/java/org/elasticsearch/index/mapper/core/StringFieldMapper.java
View file
Expand Up @@ -126,16 +126,22 @@ public StringFieldMapper build(BuilderContext context) {
// 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);
}
if (!indexOptionsSet) {
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);
Expand Down Expand Up @@ -183,16 +189,13 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
}

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,
Expand All @@ -203,6 +206,7 @@ protected StringFieldMapper(Names names, float boost, FieldType fieldType, Boole
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;
Expand All @@ -211,7 +215,7 @@ protected StringFieldMapper(Names names, float boost, FieldType fieldType, Boole

@Override
public FieldType defaultFieldType() {
return Defaults.FIELD_TYPE;
return defaultFieldType;
}

@Override
Expand Down
73 changes: 54 additions & 19 deletions src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java
View file
Expand Up @@ -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.*;
Expand Down Expand Up @@ -74,6 +77,35 @@ public void testLimit() throws Exception {
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.<String, String>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")
Expand All @@ -88,12 +120,9 @@ public void testDefaultsForAnalyzed() throws Exception {
.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
Expand All @@ -110,12 +139,14 @@ public void testDefaultsForNotAnalyzed() throws Exception {
.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

Expand All @@ -131,12 +162,14 @@ public void testDefaultsForNotAnalyzed() throws Exception {
.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

Expand All @@ -152,7 +185,9 @@ public void testDefaultsForNotAnalyzed() throws Exception {
.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
Expand Down

0 comments on commit 763cf2c

Please sign in to comment.