Skip to content

Commit

Permalink
Mappings: Remove ability to set path for _id and _routing on 2.0+ ind…
Browse files Browse the repository at this point in the history
…exes

_id and _routing now no longer support the 'path' setting on indexes
created with 2.0.  Indexes created before 2.0 still support this
setting for backcompat.

closes #6730
  • Loading branch information
rjernst committed Feb 10, 2015
1 parent 6544890 commit b3474f6
Show file tree
Hide file tree
Showing 15 changed files with 106 additions and 134 deletions.
9 changes: 9 additions & 0 deletions dev-tools/create-bwc-index.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,15 @@ def generate_index(client, version):
}
}
}
mappings['meta_fields'] = {
'_id': {
'path': 'myid'
},
'_routing': {
'path': 'myrouting'
}
}


client.indices.create(index='test', body={
'settings': {
Expand Down
28 changes: 0 additions & 28 deletions docs/reference/mapping/fields/id-field.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -25,31 +25,3 @@ using the appropriate mapping attributes:
}
--------------------------------------------------

The `_id` mapping can also be associated with a `path` that will be used
to extract the id from a different location in the source document. For
example, having the following mapping:

[source,js]
--------------------------------------------------
{
"tweet" : {
"_id" : {
"path" : "post_id"
}
}
}
--------------------------------------------------

Will cause `1` to be used as the id for:

[source,js]
--------------------------------------------------
{
"message" : "You know, for Search",
"post_id" : "1"
}
--------------------------------------------------

This does require an additional lightweight parsing step while indexing,
in order to extract the id to decide which shard the index operation
will be executed on.
36 changes: 0 additions & 36 deletions docs/reference/mapping/fields/routing-field.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -24,42 +24,6 @@ has been provided (or derived from the doc). A delete operation will be
broadcasted to all shards if no routing value is provided and `_routing`
is required.

[float]
==== path

The routing value can be provided as an external value when indexing
(and still stored as part of the document, in much the same way
`_source` is stored). But, it can also be automatically extracted from
the index doc based on a `path`. For example, having the following
mapping:

[source,js]
--------------------------------------------------
{
"comment" : {
"_routing" : {
"required" : true,
"path" : "blog.post_id"
}
}
}
--------------------------------------------------

Will cause the following doc to be routed based on the `111222` value:

[source,js]
--------------------------------------------------
{
"text" : "the comment text"
"blog" : {
"post_id" : "111222"
}
}
--------------------------------------------------

Note, using `path` without explicit routing value provided required an
additional (though quite fast) parsing phase.

[float]
==== id uniqueness

Expand Down
14 changes: 3 additions & 11 deletions src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -183,19 +183,11 @@ public Builder(String index, Settings indexSettings, RootObjectMapper.Builder bu
this.indexSettings = indexSettings;
this.builderContext = new Mapper.BuilderContext(indexSettings, new ContentPath(1));
this.rootObjectMapper = builder.build(builderContext);
IdFieldMapper idFieldMapper = new IdFieldMapper();
if (indexSettings != null) {
String idIndexed = indexSettings.get("index.mapping._id.indexed");
if (idIndexed != null && Booleans.parseBoolean(idIndexed, false)) {
FieldType fieldType = new FieldType(IdFieldMapper.Defaults.FIELD_TYPE);
fieldType.setTokenized(false);
idFieldMapper = new IdFieldMapper(fieldType);
}
}

// UID first so it will be the first stored field to load (so will benefit from "fields: []" early termination
this.rootMappers.put(UidFieldMapper.class, new UidFieldMapper());
this.rootMappers.put(IdFieldMapper.class, idFieldMapper);
this.rootMappers.put(RoutingFieldMapper.class, new RoutingFieldMapper());
this.rootMappers.put(IdFieldMapper.class, new IdFieldMapper(indexSettings));
this.rootMappers.put(RoutingFieldMapper.class, new RoutingFieldMapper(indexSettings));
// add default mappers, order is important (for example analyzer should come before the rest to set context.analyzer)
this.rootMappers.put(SizeFieldMapper.class, new SizeFieldMapper(indexSettings));
this.rootMappers.put(IndexFieldMapper.class, new IndexFieldMapper());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.lucene.queries.TermsFilter;
import org.apache.lucene.search.*;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.Version;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.lucene.BytesRefs;
Expand Down Expand Up @@ -58,7 +59,7 @@
import static org.elasticsearch.index.mapper.core.TypeParsers.parseField;

/**
*
*
*/
public class IdFieldMapper extends AbstractFieldMapper<String> implements InternalMapper, RootMapper {

Expand Down Expand Up @@ -115,7 +116,7 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
Map.Entry<String, Object> entry = iterator.next();
String fieldName = Strings.toUnderscoreCase(entry.getKey());
Object fieldNode = entry.getValue();
if (fieldName.equals("path")) {
if (fieldName.equals("path") && parserContext.indexVersionCreated().before(Version.V_2_0_0)) {
builder.path(fieldNode.toString());
iterator.remove();
}
Expand All @@ -125,17 +126,10 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
}

private final String path;
private final boolean writePre20Settings;

public IdFieldMapper() {
this(new FieldType(Defaults.FIELD_TYPE));
}

public IdFieldMapper(FieldType fieldType) {
this(Defaults.NAME, Defaults.INDEX_NAME, fieldType, null);
}

protected IdFieldMapper(String name, String indexName, FieldType fieldType, Boolean docValues) {
this(name, indexName, Defaults.BOOST, fieldType, docValues, Defaults.PATH, null, null, null, ImmutableSettings.EMPTY);
public IdFieldMapper(Settings indexSettings) {
this(Defaults.NAME, Defaults.INDEX_NAME, Defaults.BOOST, idFieldType(indexSettings), null, Defaults.PATH, null, null, null, indexSettings);
}

protected IdFieldMapper(String name, String indexName, float boost, FieldType fieldType, Boolean docValues, String path,
Expand All @@ -144,6 +138,15 @@ protected IdFieldMapper(String name, String indexName, float boost, FieldType fi
super(new Names(name, indexName, indexName, name), boost, fieldType, docValues, Lucene.KEYWORD_ANALYZER,
Lucene.KEYWORD_ANALYZER, postingsProvider, docValuesProvider, null, null, fieldDataSettings, indexSettings);
this.path = path;
this.writePre20Settings = Version.indexCreated(indexSettings).before(Version.V_2_0_0);
}

private static FieldType idFieldType(Settings indexSettings) {
FieldType fieldType = new FieldType(Defaults.FIELD_TYPE);
if (indexSettings.getAsBoolean("index.mapping._id.indexed", true) == false) {
fieldType.setTokenized(false);
}
return fieldType;
}

public String path() {
Expand Down Expand Up @@ -351,7 +354,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (includeDefaults || fieldType.indexOptions() != Defaults.FIELD_TYPE.indexOptions()) {
builder.field("index", indexTokenizeOptionToString(fieldType.indexOptions() != IndexOptions.NONE, fieldType.tokenized()));
}
if (includeDefaults || path != Defaults.PATH) {
if (writePre20Settings && (includeDefaults || path != Defaults.PATH)) {
builder.field("path", path);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.lucene.document.Field;
import org.apache.lucene.document.FieldType;
import org.apache.lucene.index.IndexOptions;
import org.elasticsearch.Version;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.lucene.Lucene;
Expand Down Expand Up @@ -107,7 +108,7 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
if (fieldName.equals("required")) {
builder.required(nodeBooleanValue(fieldNode));
iterator.remove();
} else if (fieldName.equals("path")) {
} else if (fieldName.equals("path") && parserContext.indexVersionCreated().before(Version.V_2_0_0)) {
builder.path(fieldNode.toString());
iterator.remove();
}
Expand All @@ -118,11 +119,11 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext


private boolean required;

private final String path;
private final boolean writePre20Settings;

public RoutingFieldMapper() {
this(new FieldType(Defaults.FIELD_TYPE), Defaults.REQUIRED, Defaults.PATH, null, null, null, ImmutableSettings.EMPTY);
public RoutingFieldMapper(Settings indexSettings) {
this(Defaults.FIELD_TYPE, Defaults.REQUIRED, Defaults.PATH, null, null, null, indexSettings);
}

protected RoutingFieldMapper(FieldType fieldType, boolean required, String path, PostingsFormatProvider postingsProvider,
Expand All @@ -131,6 +132,7 @@ protected RoutingFieldMapper(FieldType fieldType, boolean required, String path,
Lucene.KEYWORD_ANALYZER, postingsProvider, docValuesProvider, null, null, fieldDataSettings, indexSettings);
this.required = required;
this.path = path;
this.writePre20Settings = Version.indexCreated(indexSettings).before(Version.V_2_0_0);
}

@Override
Expand Down Expand Up @@ -234,7 +236,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (includeDefaults || required != Defaults.REQUIRED) {
builder.field("required", required);
}
if (includeDefaults || path != Defaults.PATH) {
if (writePre20Settings && (includeDefaults || path != Defaults.PATH)) {
builder.field("path", path);
}
builder.endObject();
Expand Down
8 changes: 6 additions & 2 deletions src/test/java/org/elasticsearch/document/BulkTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.google.common.base.Charsets;

import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.alias.Alias;
import org.elasticsearch.action.bulk.BulkItemResponse;
import org.elasticsearch.action.bulk.BulkRequest;
Expand All @@ -35,6 +36,7 @@
import org.elasticsearch.action.update.UpdateRequest;
import org.elasticsearch.action.update.UpdateRequestBuilder;
import org.elasticsearch.action.update.UpdateResponse;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.xcontent.XContentBuilder;
Expand Down Expand Up @@ -594,7 +596,8 @@ public void preParsingSourceDueToRoutingShouldNotBreakCompleteBulkRequest() thro
.endObject()
.endObject()
.endObject();
assertAcked(prepareCreate("test").addMapping("type", builder));
assertAcked(prepareCreate("test").addMapping("type", builder)
.setSettings(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2_ID));
ensureYellow("test");

String brokenBuildRequestData = "{\"index\": {} }\n" +
Expand All @@ -620,7 +623,8 @@ public void preParsingSourceDueToIdShouldNotBreakCompleteBulkRequest() throws Ex
.endObject()
.endObject()
.endObject();
assertAcked(prepareCreate("test").addMapping("type", builder));
assertAcked(prepareCreate("test").addMapping("type", builder)
.setSettings(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2_ID));
ensureYellow("test");

String brokenBuildRequestData = "{\"index\": {} }\n" +
Expand Down
23 changes: 12 additions & 11 deletions src/test/java/org/elasticsearch/index/mapper/id/IdMappingTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@

package org.elasticsearch.index.mapper.id;

import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
Expand All @@ -28,15 +32,13 @@
import org.elasticsearch.index.mapper.internal.IdFieldMapper;
import org.elasticsearch.index.mapper.internal.UidFieldMapper;
import org.elasticsearch.test.ElasticsearchSingleNodeTest;
import org.junit.Test;

import static org.hamcrest.Matchers.*;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;

/**
*/
public class IdMappingTests extends ElasticsearchSingleNodeTest {

@Test

public void simpleIdTests() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.endObject().endObject().string();
Expand Down Expand Up @@ -68,8 +70,7 @@ public void simpleIdTests() throws Exception {
assertThat(doc.rootDoc().get(UidFieldMapper.NAME), notNullValue());
assertThat(doc.rootDoc().get(IdFieldMapper.NAME), nullValue());
}

@Test

public void testIdIndexed() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_id").field("index", "not_analyzed").endObject()
Expand All @@ -93,13 +94,13 @@ public void testIdIndexed() throws Exception {
assertThat(doc.rootDoc().get(UidFieldMapper.NAME), notNullValue());
assertThat(doc.rootDoc().get(IdFieldMapper.NAME), notNullValue());
}

@Test

public void testIdPath() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_id").field("path", "my_path").endObject()
.endObject().endObject().string();
DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);
Settings settings = ImmutableSettings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2_ID).build();
DocumentMapper docMapper = createIndex("test", settings).mapperService().documentMapperParser().parse(mapping);

// serialize the id mapping
XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,11 @@ public void testSetValues() throws Exception {
.startObject("_routing")
.field("store", "no")
.field("index", "no")
.field("path", "route")
.endObject()
.endObject().endObject().string();
DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);
assertThat(docMapper.routingFieldMapper().fieldType().stored(), equalTo(false));
assertEquals(IndexOptions.NONE, docMapper.routingFieldMapper().fieldType().indexOptions());
assertThat(docMapper.routingFieldMapper().path(), equalTo("route"));
}

@Test
Expand Down
Loading

0 comments on commit b3474f6

Please sign in to comment.