From da707b6f32e98943fa6caf3704119400cbef1c7c Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 14 Jan 2014 13:39:52 +0100 Subject: [PATCH] Remove `omit_term_freq_and_positions` for new indices `omit_term_freq_and_positions` was deprecated in `0.20` and is not documented anymore. We should reject indices that are created with this option in the future. Closes #4722 --- .../index/mapper/DocumentMapperParser.java | 17 ++++---- .../elasticsearch/index/mapper/Mapper.java | 9 +++- .../index/mapper/core/TypeParsers.java | 7 +++- .../count/query/SimpleQueryTests.java | 19 --------- .../index/mapper/MapperTestUtils.java | 2 +- .../search/query/SimpleQueryTests.java | 42 ++++++++++++------- 6 files changed, 51 insertions(+), 45 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java b/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java index df985b3c70b..68af75d70e7 100644 --- a/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java +++ b/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java @@ -21,13 +21,14 @@ package org.elasticsearch.index.mapper; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.compress.CompressedString; import org.elasticsearch.common.geo.ShapesAvailability; -import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentHelper; @@ -64,16 +65,11 @@ public class DocumentMapperParser extends AbstractIndexComponent { private final RootObjectMapper.TypeParser rootObjectTypeParser = new RootObjectMapper.TypeParser(); private final Object typeParsersMutex = new Object(); + private final Version indexVersionCreated; private volatile ImmutableMap typeParsers; private volatile ImmutableMap rootTypeParsers; - public DocumentMapperParser(Index index, AnalysisService analysisService, PostingsFormatService postingsFormatService, - DocValuesFormatService docValuesFormatService, SimilarityLookupService similarityLookupService) { - this(index, ImmutableSettings.Builder.EMPTY_SETTINGS, analysisService, postingsFormatService, docValuesFormatService, - similarityLookupService); - } - public DocumentMapperParser(Index index, @IndexSettings Settings indexSettings, AnalysisService analysisService, PostingsFormatService postingsFormatService, DocValuesFormatService docValuesFormatService, SimilarityLookupService similarityLookupService) { @@ -123,6 +119,9 @@ public class DocumentMapperParser extends AbstractIndexComponent { .put(VersionFieldMapper.NAME, new VersionFieldMapper.TypeParser()) .put(IdFieldMapper.NAME, new IdFieldMapper.TypeParser()) .immutableMap(); + assert indexSettings.get(IndexMetaData.SETTING_UUID) == null // if the UUDI is there the index has actually been created otherwise this might be a test + || indexSettings.getAsVersion(IndexMetaData.SETTING_VERSION_CREATED, null) != null : IndexMetaData.SETTING_VERSION_CREATED + " not set in IndexSettings"; + indexVersionCreated = indexSettings.getAsVersion(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT); } public void putTypeParser(String type, Mapper.TypeParser typeParser) { @@ -142,7 +141,7 @@ public class DocumentMapperParser extends AbstractIndexComponent { } public Mapper.TypeParser.ParserContext parserContext() { - return new Mapper.TypeParser.ParserContext(postingsFormatService, docValuesFormatService, analysisService, similarityLookupService, typeParsers); + return new Mapper.TypeParser.ParserContext(postingsFormatService, docValuesFormatService, analysisService, similarityLookupService, typeParsers, indexVersionCreated); } public DocumentMapper parse(String source) throws MapperParsingException { @@ -199,8 +198,8 @@ public class DocumentMapperParser extends AbstractIndexComponent { } } - Mapper.TypeParser.ParserContext parserContext = new Mapper.TypeParser.ParserContext(postingsFormatService, docValuesFormatService, analysisService, similarityLookupService, typeParsers); + Mapper.TypeParser.ParserContext parserContext = parserContext(); DocumentMapper.Builder docBuilder = doc(index.name(), indexSettings, (RootObjectMapper.Builder) rootObjectTypeParser.parse(type, mapping, parserContext)); for (Map.Entry entry : mapping.entrySet()) { diff --git a/src/main/java/org/elasticsearch/index/mapper/Mapper.java b/src/main/java/org/elasticsearch/index/mapper/Mapper.java index 357db283809..6b1d65dee85 100644 --- a/src/main/java/org/elasticsearch/index/mapper/Mapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/Mapper.java @@ -99,14 +99,17 @@ public interface Mapper extends ToXContent { private final ImmutableMap typeParsers; + private final Version indexVersionCreated; + public ParserContext(PostingsFormatService postingsFormatService, DocValuesFormatService docValuesFormatService, AnalysisService analysisService, SimilarityLookupService similarityLookupService, - ImmutableMap typeParsers) { + ImmutableMap typeParsers, Version indexVersionCreated) { this.postingsFormatService = postingsFormatService; this.docValuesFormatService = docValuesFormatService; this.analysisService = analysisService; this.similarityLookupService = similarityLookupService; this.typeParsers = typeParsers; + this.indexVersionCreated = indexVersionCreated; } public AnalysisService analysisService() { @@ -128,6 +131,10 @@ public interface Mapper extends ToXContent { public TypeParser typeParser(String type) { return typeParsers.get(Strings.toUnderscoreCase(type)); } + + public Version indexVersionCreated() { + return indexVersionCreated; + } } Mapper.Builder parse(String name, Map node, ParserContext parserContext) throws MapperParsingException; diff --git a/src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java b/src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java index c142acae2a6..5edd57cdad2 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java @@ -21,6 +21,7 @@ package org.elasticsearch.index.mapper.core; import org.apache.lucene.index.FieldInfo.IndexOptions; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.Version; import org.elasticsearch.common.Strings; import org.elasticsearch.common.joda.FormatDateTimeFormatter; import org.elasticsearch.common.joda.Joda; @@ -198,8 +199,12 @@ public class TypeParsers { } } } else if (propName.equals("omit_term_freq_and_positions")) { + final IndexOptions op = nodeBooleanValue(propNode) ? IndexOptions.DOCS_ONLY : IndexOptions.DOCS_AND_FREQS_AND_POSITIONS; + if (parserContext.indexVersionCreated().onOrAfter(Version.V_1_0_0)) { + throw new ElasticsearchParseException("'omit_term_freq_and_positions' is not supported anymore - use ['index_options' : '" + op.name() + "'] instead"); + } // deprecated option for BW compat - builder.indexOptions(nodeBooleanValue(propNode) ? IndexOptions.DOCS_ONLY : IndexOptions.DOCS_AND_FREQS_AND_POSITIONS); + builder.indexOptions(op); } else if (propName.equals("index_options")) { builder.indexOptions(nodeIndexOptionValue(propNode)); } else if (propName.equals("analyzer")) { diff --git a/src/test/java/org/elasticsearch/count/query/SimpleQueryTests.java b/src/test/java/org/elasticsearch/count/query/SimpleQueryTests.java index 2ad65faa668..ecb1cf7e9dd 100644 --- a/src/test/java/org/elasticsearch/count/query/SimpleQueryTests.java +++ b/src/test/java/org/elasticsearch/count/query/SimpleQueryTests.java @@ -130,25 +130,6 @@ public class SimpleQueryTests extends ElasticsearchIntegrationTest { assertHitCount(countResponse, 3l); } - @Test - public void testOmitTermFreqsAndPositions() throws Exception { - // backwards compat test! - assertAcked(prepareCreate("test") - .addMapping("type1", "field1", "type=string,omit_term_freq_and_positions=true") - .setSettings(SETTING_NUMBER_OF_SHARDS, 1)); - - indexRandom(true, client().prepareIndex("test", "type1", "1").setSource("field1", "quick brown fox", "field2", "quick brown fox"), - client().prepareIndex("test", "type1", "2").setSource("field1", "quick lazy huge brown fox", "field2", "quick lazy huge brown fox")); - - CountResponse countResponse = client().prepareCount().setQuery(QueryBuilders.matchQuery("field2", "quick brown").type(Type.PHRASE).slop(0)).get(); - assertHitCount(countResponse, 1l); - try { - client().prepareCount().setQuery(QueryBuilders.matchQuery("field1", "quick brown").type(Type.PHRASE).slop(0)).get(); - } catch (SearchPhaseExecutionException e) { - assertTrue(e.getMessage().endsWith("IllegalStateException[field \"field1\" was indexed without position data; cannot run PhraseQuery (term=quick)]; }")); - } - } - @Test public void queryStringAnalyzedWildcard() throws Exception { assertAcked(prepareCreate("test").setSettings(SETTING_NUMBER_OF_SHARDS, 1)); diff --git a/src/test/java/org/elasticsearch/index/mapper/MapperTestUtils.java b/src/test/java/org/elasticsearch/index/mapper/MapperTestUtils.java index f91655508e8..5fb90e55ac2 100644 --- a/src/test/java/org/elasticsearch/index/mapper/MapperTestUtils.java +++ b/src/test/java/org/elasticsearch/index/mapper/MapperTestUtils.java @@ -45,7 +45,7 @@ import org.elasticsearch.indices.fielddata.breaker.DummyCircuitBreakerService; public class MapperTestUtils { public static DocumentMapperParser newParser() { - return new DocumentMapperParser(new Index("test"), newAnalysisService(), new PostingsFormatService(new Index("test")), + return new DocumentMapperParser(new Index("test"), ImmutableSettings.Builder.EMPTY_SETTINGS, newAnalysisService(), new PostingsFormatService(new Index("test")), new DocValuesFormatService(new Index("test")), newSimilarityLookupService()); } diff --git a/src/test/java/org/elasticsearch/search/query/SimpleQueryTests.java b/src/test/java/org/elasticsearch/search/query/SimpleQueryTests.java index 53c31926d79..4f852bbc8ac 100644 --- a/src/test/java/org/elasticsearch/search/query/SimpleQueryTests.java +++ b/src/test/java/org/elasticsearch/search/query/SimpleQueryTests.java @@ -21,13 +21,16 @@ package org.elasticsearch.search.query; import org.apache.lucene.util.English; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.Version; import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.action.search.ShardSearchFailure; +import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.query.*; import org.elasticsearch.index.query.CommonTermsQueryBuilder.Operator; import org.elasticsearch.index.query.MatchQueryBuilder.Type; @@ -325,21 +328,32 @@ public class SimpleQueryTests extends ElasticsearchIntegrationTest { @Test public void testOmitTermFreqsAndPositions() throws Exception { - // backwards compat test! - assertAcked(client().admin().indices().prepareCreate("test") - .addMapping("type1", "field1", "type=string,omit_term_freq_and_positions=true") - .setSettings(SETTING_NUMBER_OF_SHARDS, 1)); + Version version = Version.CURRENT; + int iters = atLeast(10); + for (int i = 0; i < iters; i++) { + try { + // backwards compat test! + assertAcked(client().admin().indices().prepareCreate("test") + .addMapping("type1", "field1", "type=string,omit_term_freq_and_positions=true") + .setSettings(SETTING_NUMBER_OF_SHARDS, 1, IndexMetaData.SETTING_VERSION_CREATED, version.id)); + assertThat(version.onOrAfter(Version.V_1_0_0), equalTo(false)); + indexRandom(true, client().prepareIndex("test", "type1", "1").setSource("field1", "quick brown fox", "field2", "quick brown fox"), + client().prepareIndex("test", "type1", "2").setSource("field1", "quick lazy huge brown fox", "field2", "quick lazy huge brown fox")); - indexRandom(true, client().prepareIndex("test", "type1", "1").setSource("field1", "quick brown fox", "field2", "quick brown fox"), - client().prepareIndex("test", "type1", "2").setSource("field1", "quick lazy huge brown fox", "field2", "quick lazy huge brown fox")); - - SearchResponse searchResponse = client().prepareSearch().setQuery(matchQuery("field2", "quick brown").type(MatchQueryBuilder.Type.PHRASE).slop(0)).get(); - assertHitCount(searchResponse, 1l); - try { - client().prepareSearch().setQuery(matchQuery("field1", "quick brown").type(MatchQueryBuilder.Type.PHRASE).slop(0)).get(); - fail("SearchPhaseExecutionException should have been thrown"); - } catch (SearchPhaseExecutionException e) { - assertTrue(e.getMessage().endsWith("IllegalStateException[field \"field1\" was indexed without position data; cannot run PhraseQuery (term=quick)]; }")); + SearchResponse searchResponse = client().prepareSearch().setQuery(matchQuery("field2", "quick brown").type(MatchQueryBuilder.Type.PHRASE).slop(0)).get(); + assertHitCount(searchResponse, 1l); + try { + client().prepareSearch().setQuery(matchQuery("field1", "quick brown").type(MatchQueryBuilder.Type.PHRASE).slop(0)).get(); + fail("SearchPhaseExecutionException should have been thrown"); + } catch (SearchPhaseExecutionException e) { + assertTrue(e.getMessage().endsWith("IllegalStateException[field \"field1\" was indexed without position data; cannot run PhraseQuery (term=quick)]; }")); + } + wipeIndices("test"); + } catch (MapperParsingException ex) { + assertThat(version.toString(), version.onOrAfter(Version.V_1_0_0), equalTo(true)); + assertThat(ex.getCause().getMessage(), equalTo("'omit_term_freq_and_positions' is not supported anymore - use ['index_options' : 'DOCS_ONLY'] instead")); + } + version = randomVersion(); } }