From 4e1f72552cda4b3335c46c34af98150228ee5f7d Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 8 Jun 2022 14:02:25 +0100 Subject: [PATCH] Don't run include_in_parent when in copy_to context (#87123) We changed how copy_to is implemented in #79922, which moved the handling of dots in field names into a specialised parser. Unfortunately, while doing this we added a bug whereby every time a copy_to directive is processed for a nested field, the nested field's include_in_parent logic would be run, meaning that the parent would end up with multiple copies of the nested child's fields. This commit fixes this by only running include_in_parent when the parser is not in a copy_to context. It also fixes another bug that meant the parent document would contain multiple copies of the ID field. Fixes #87036 --- docs/changelog/87123.yaml | 6 +++ .../index/mapper/DocumentParser.java | 13 +++-- .../index/mapper/CopyToMapperTests.java | 48 +++++++++++++++++++ 3 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 docs/changelog/87123.yaml diff --git a/docs/changelog/87123.yaml b/docs/changelog/87123.yaml new file mode 100644 index 000000000000..2d04b908bfd3 --- /dev/null +++ b/docs/changelog/87123.yaml @@ -0,0 +1,6 @@ +pr: 87123 +summary: Don't run `include_in_parent` when in `copy_to` context +area: Mapping +type: bug +issues: + - 87036 diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 4cfdb38bbc09..3a485c60ad55 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -278,7 +278,7 @@ static void parseObjectOrNested(DocumentParserContext context, ObjectMapper mapp innerParseObject(context, mapper); // restore the enable path flag if (mapper.isNested()) { - nested(context, (NestedObjectMapper) mapper); + copyNestedFields(context, (NestedObjectMapper) mapper); } } @@ -344,7 +344,14 @@ private static void throwEOF(ObjectMapper mapper, String currentFieldName) { ); } - private static void nested(DocumentParserContext context, NestedObjectMapper nested) { + private static void copyNestedFields(DocumentParserContext context, NestedObjectMapper nested) { + if (context.isWithinCopyTo()) { + // Only process the nested document after we've finished parsing the actual + // doc; we can't copy_to outside of the current nested context, so if we are + // in a copy_to context then we're adding data within the doc and we haven't + // finished parsing yet. + return; + } LuceneDocument nestedDoc = context.doc(); LuceneDocument parentDoc = nestedDoc.getParent(); Version indexVersion = context.indexSettings().getIndexVersionCreated(); @@ -363,7 +370,7 @@ private static void nested(DocumentParserContext context, NestedObjectMapper nes private static void addFields(Version indexCreatedVersion, LuceneDocument nestedDoc, LuceneDocument rootDoc) { String nestedPathFieldName = NestedPathFieldMapper.name(indexCreatedVersion); for (IndexableField field : nestedDoc.getFields()) { - if (field.name().equals(nestedPathFieldName) == false) { + if (field.name().equals(nestedPathFieldName) == false && field.name().equals(IdFieldMapper.NAME) == false) { rootDoc.add(field); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CopyToMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CopyToMapperTests.java index 53fe6b1228ad..70156768d63f 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CopyToMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CopyToMapperTests.java @@ -744,4 +744,52 @@ public void testCopyToGeoPoint() throws Exception { ); } } + + public void testCopyToMultipleNested() throws IOException { + + // Don't copy values beyond a single step + + DocumentMapper documentMapper = createDocumentMapper(""" + { "_doc" : { "properties" : { + "_all" : { "type" : "text" }, + "du" : { + "type" : "nested", + "include_in_root" : "true", + "properties" : { + "_all" : { "type" : "text" }, + "bc" : { + "type" : "nested", + "include_in_parent" : "true", + "properties" : { + "_all" : { "type" : "text" }, + "bc4" : { + "type" : "nested", + "include_in_parent" : "true", + "properties" : { + "_all" : { "type" : "text" }, + "area" : { + "type" : "text", + "copy_to" : [ + "_all", + "du._all", + "du.bc._all", + "du.bc.bc4._all" + ] + } + } + } + } + } + } + } + }}}"""); + + ParsedDocument doc = documentMapper.parse(source(""" + { "du" : { "bc" : [ { "bc4": { "area" : "foo" } }, { "bc4" : { "area" : "bar" } } ] } } + """)); + + assertEquals(1, doc.rootDoc().getFields("_id").length); + assertEquals(2, doc.rootDoc().getFields("du._all").length); + + } }