diff options
4 files changed, 126 insertions, 22 deletions
diff --git a/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 489f4702bc..c2de26c96b 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -424,15 +424,33 @@ final class DocumentParser { context = context.createNestedContext(mapper.fullPath()); ParseContext.Document nestedDoc = context.doc(); ParseContext.Document parentDoc = nestedDoc.getParent(); - // pre add the uid field if possible (id was already provided) - IndexableField uidField = parentDoc.getField(UidFieldMapper.NAME); - if (uidField != null) { - // we don't need to add it as a full uid field in nested docs, since we don't need versioning - // we also rely on this for UidField#loadVersion - // this is a deeply nested field - nestedDoc.add(new Field(UidFieldMapper.NAME, uidField.stringValue(), UidFieldMapper.Defaults.NESTED_FIELD_TYPE)); + // We need to add the uid or id to this nested Lucene document too, + // If we do not do this then when a document gets deleted only the root Lucene document gets deleted and + // not the nested Lucene documents! Besides the fact that we would have zombie Lucene documents, the ordering of + // documents inside the Lucene index (document blocks) will be incorrect, as nested documents of different root + // documents are then aligned with other root documents. This will lead tothe nested query, sorting, aggregations + // and inner hits to fail or yield incorrect results. + if (context.mapperService().getIndexSettings().isSingleType()) { + IndexableField idField = parentDoc.getField(IdFieldMapper.NAME); + if (idField != null) { + // We just need to store the id as indexed field, so that IndexWriter#deleteDocuments(term) can then + // delete it when the root document is deleted too. + nestedDoc.add(new Field(IdFieldMapper.NAME, idField.stringValue(), IdFieldMapper.Defaults.NESTED_FIELD_TYPE)); + } else { + throw new IllegalStateException("The root document of a nested document should have an id field"); + } + } else { + IndexableField uidField = parentDoc.getField(UidFieldMapper.NAME); + if (uidField != null) { + /// We just need to store the uid as indexed field, so that IndexWriter#deleteDocuments(term) can then + // delete it when the root document is deleted too. + nestedDoc.add(new Field(UidFieldMapper.NAME, uidField.stringValue(), UidFieldMapper.Defaults.NESTED_FIELD_TYPE)); + } else { + throw new IllegalStateException("The root document of a nested document should have an uid field"); + } } + // the type of the nested doc starts with __, so we can identify that its a nested one in filters // note, we don't prefix it with the type of the doc since it allows us to execute a nested query // across types (for example, with similar nested objects) diff --git a/core/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java index a9a765f1c3..813a546aae 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java @@ -52,6 +52,7 @@ public class IdFieldMapper extends MetadataFieldMapper { public static final String NAME = IdFieldMapper.NAME; public static final MappedFieldType FIELD_TYPE = new IdFieldType(); + public static final MappedFieldType NESTED_FIELD_TYPE; static { FIELD_TYPE.setTokenized(false); @@ -62,6 +63,10 @@ public class IdFieldMapper extends MetadataFieldMapper { FIELD_TYPE.setSearchAnalyzer(Lucene.KEYWORD_ANALYZER); FIELD_TYPE.setName(NAME); FIELD_TYPE.freeze(); + + NESTED_FIELD_TYPE = FIELD_TYPE.clone(); + NESTED_FIELD_TYPE.setStored(false); + NESTED_FIELD_TYPE.freeze(); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java index 49864768ed..d3d099672b 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.IndexService; @@ -148,6 +149,92 @@ public class DocumentParserTests extends ESSingleNodeTestCase { e.getMessage()); } + public void testNestedHaveIdAndTypeFields() throws Exception { + DocumentMapperParser mapperParser1 = createIndex("index1", Settings.builder() + .put("index.mapping.single_type", false).build() + ).mapperService().documentMapperParser(); + DocumentMapperParser mapperParser2 = createIndex("index2", Settings.builder() + .put("index.mapping.single_type", true).build() + ).mapperService().documentMapperParser(); + + XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties"); + { + mapping.startObject("foo"); + mapping.field("type", "nested"); + { + mapping.startObject("properties"); + { + + mapping.startObject("bar"); + mapping.field("type", "keyword"); + mapping.endObject(); + } + mapping.endObject(); + } + mapping.endObject(); + } + { + mapping.startObject("baz"); + mapping.field("type", "keyword"); + mapping.endObject(); + } + mapping.endObject().endObject().endObject(); + DocumentMapper mapper1 = mapperParser1.parse("type", new CompressedXContent(mapping.string())); + DocumentMapper mapper2 = mapperParser2.parse("type", new CompressedXContent(mapping.string())); + + XContentBuilder doc = XContentFactory.jsonBuilder().startObject(); + { + doc.startArray("foo"); + { + doc.startObject(); + doc.field("bar", "value1"); + doc.endObject(); + } + doc.endArray(); + doc.field("baz", "value2"); + } + doc.endObject(); + + // Verify in the case where multiple types are allowed that the _uid field is added to nested documents: + ParsedDocument result = mapper1.parse(SourceToParse.source("index1", "type", "1", doc.bytes(), XContentType.JSON)); + assertEquals(2, result.docs().size()); + // Nested document: + assertNull(result.docs().get(0).getField(IdFieldMapper.NAME)); + assertNotNull(result.docs().get(0).getField(UidFieldMapper.NAME)); + assertEquals("type#1", result.docs().get(0).getField(UidFieldMapper.NAME).stringValue()); + assertEquals(UidFieldMapper.Defaults.NESTED_FIELD_TYPE, result.docs().get(0).getField(UidFieldMapper.NAME).fieldType()); + assertNotNull(result.docs().get(0).getField(TypeFieldMapper.NAME)); + assertEquals("__foo", result.docs().get(0).getField(TypeFieldMapper.NAME).stringValue()); + assertEquals("value1", result.docs().get(0).getField("foo.bar").binaryValue().utf8ToString()); + // Root document: + assertNull(result.docs().get(1).getField(IdFieldMapper.NAME)); + assertNotNull(result.docs().get(1).getField(UidFieldMapper.NAME)); + assertEquals("type#1", result.docs().get(1).getField(UidFieldMapper.NAME).stringValue()); + assertEquals(UidFieldMapper.Defaults.FIELD_TYPE, result.docs().get(1).getField(UidFieldMapper.NAME).fieldType()); + assertNotNull(result.docs().get(1).getField(TypeFieldMapper.NAME)); + assertEquals("type", result.docs().get(1).getField(TypeFieldMapper.NAME).stringValue()); + assertEquals("value2", result.docs().get(1).getField("baz").binaryValue().utf8ToString()); + + // Verify in the case where only a single type is allowed that the _id field is added to nested documents: + result = mapper2.parse(SourceToParse.source("index2", "type", "1", doc.bytes(), XContentType.JSON)); + assertEquals(2, result.docs().size()); + // Nested document: + assertNull(result.docs().get(0).getField(UidFieldMapper.NAME)); + assertNotNull(result.docs().get(0).getField(IdFieldMapper.NAME)); + assertEquals("1", result.docs().get(0).getField(IdFieldMapper.NAME).stringValue()); + assertEquals(IdFieldMapper.Defaults.NESTED_FIELD_TYPE, result.docs().get(0).getField(IdFieldMapper.NAME).fieldType()); + assertNotNull(result.docs().get(0).getField(TypeFieldMapper.NAME)); + assertEquals("__foo", result.docs().get(0).getField(TypeFieldMapper.NAME).stringValue()); + assertEquals("value1", result.docs().get(0).getField("foo.bar").binaryValue().utf8ToString()); + // Root document: + assertNull(result.docs().get(1).getField(UidFieldMapper.NAME)); + assertNotNull(result.docs().get(1).getField(IdFieldMapper.NAME)); + assertEquals("1", result.docs().get(1).getField(IdFieldMapper.NAME).stringValue()); + assertEquals(IdFieldMapper.Defaults.FIELD_TYPE, result.docs().get(1).getField(IdFieldMapper.NAME).fieldType()); + assertNull(result.docs().get(1).getField(TypeFieldMapper.NAME)); + assertEquals("value2", result.docs().get(1).getField("baz").binaryValue().utf8ToString()); + } + public void testPropagateDynamicWithExistingMapper() throws Exception { DocumentMapperParser mapperParser = createIndex("test").mapperService().documentMapperParser(); String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") @@ -639,7 +726,7 @@ public class DocumentParserTests extends ESSingleNodeTestCase { .value(0) .value(1) .endArray().endObject().bytes(); - MapperParsingException exception = expectThrows(MapperParsingException.class, + MapperParsingException exception = expectThrows(MapperParsingException.class, () -> mapper.parse(SourceToParse.source("test", "type", "1", bytes, XContentType.JSON))); assertEquals("Could not dynamically add mapping for field [foo.bar.baz]. " + "Existing mapping for [foo] must be of type object but found [long].", exception.getMessage()); @@ -758,7 +845,7 @@ public class DocumentParserTests extends ESSingleNodeTestCase { BytesReference bytes = XContentFactory.jsonBuilder() .startObject().field("foo.bar.baz", 0) .endObject().bytes(); - MapperParsingException exception = expectThrows(MapperParsingException.class, + MapperParsingException exception = expectThrows(MapperParsingException.class, () -> mapper.parse(SourceToParse.source("test", "type", "1", bytes, XContentType.JSON))); assertEquals("Could not dynamically add mapping for field [foo.bar.baz]. " + "Existing mapping for [foo] must be of type object but found [long].", exception.getMessage()); @@ -880,7 +967,7 @@ public class DocumentParserTests extends ESSingleNodeTestCase { BytesReference bytes = XContentFactory.jsonBuilder().startObject().startObject("foo.bar.baz").field("a", 0).endObject().endObject() .bytes(); - MapperParsingException exception = expectThrows(MapperParsingException.class, + MapperParsingException exception = expectThrows(MapperParsingException.class, () -> mapper.parse(SourceToParse.source("test", "type", "1", bytes, XContentType.JSON))); assertEquals("Could not dynamically add mapping for field [foo.bar.baz]. " @@ -1017,7 +1104,7 @@ public class DocumentParserTests extends ESSingleNodeTestCase { .field("test2", "value2") .startObject("inner").field("inner_field", "inner_value").endObject() .endObject() - .bytes(), + .bytes(), XContentType.JSON)); assertThat(doc.rootDoc().get("test1"), equalTo("value1")); @@ -1036,7 +1123,7 @@ public class DocumentParserTests extends ESSingleNodeTestCase { .field("test2", "value2") .startObject("inner").field("inner_field", "inner_value").endObject() .endObject().endObject() - .bytes(), + .bytes(), XContentType.JSON)); assertThat(doc.rootDoc().get("type.test1"), equalTo("value1")); @@ -1056,7 +1143,7 @@ public class DocumentParserTests extends ESSingleNodeTestCase { .field("test2", "value2") .startObject("inner").field("inner_field", "inner_value").endObject() .endObject() - .bytes(), + .bytes(), XContentType.JSON)); assertThat(doc.rootDoc().get("type"), equalTo("value_type")); @@ -1077,7 +1164,7 @@ public class DocumentParserTests extends ESSingleNodeTestCase { .field("test2", "value2") .startObject("inner").field("inner_field", "inner_value").endObject() .endObject().endObject() - .bytes(), + .bytes(), XContentType.JSON)); assertThat(doc.rootDoc().get("type.type"), equalTo("value_type")); @@ -1098,7 +1185,7 @@ public class DocumentParserTests extends ESSingleNodeTestCase { .field("test2", "value2") .startObject("inner").field("inner_field", "inner_value").endObject() .endObject() - .bytes(), + .bytes(), XContentType.JSON)); // in this case, we analyze the type object as the actual document, and ignore the other same level fields diff --git a/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java b/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java index 316e83ad1b..3e4792690a 100644 --- a/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java +++ b/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java @@ -57,9 +57,7 @@ import static org.hamcrest.Matchers.startsWith; public class SimpleNestedIT extends ESIntegTestCase { public void testSimpleNested() throws Exception { assertAcked(prepareCreate("test") - .setSettings("index.mapping.single_type", false) - .addMapping("type1", "nested1", "type=nested") - .addMapping("type2", "nested1", "type=nested")); + .addMapping("type1", "nested1", "type=nested")); ensureGreen(); // check on no data, see it works @@ -158,10 +156,6 @@ public class SimpleNestedIT extends ESIntegTestCase { searchResponse = client().prepareSearch("test").setQuery(nestedQuery("nested1", termQuery("nested1.n_field1", "n_value1_1"), ScoreMode.Avg)).execute().actionGet(); assertNoFailures(searchResponse); assertThat(searchResponse.getHits().getTotalHits(), equalTo(1L)); - - searchResponse = client().prepareSearch("test").setTypes("type1", "type2").setQuery(nestedQuery("nested1", termQuery("nested1.n_field1", "n_value1_1"), ScoreMode.Avg)).execute().actionGet(); - assertNoFailures(searchResponse); - assertThat(searchResponse.getHits().getTotalHits(), equalTo(1L)); } public void testMultiNested() throws Exception { |