Skip to content

Commit

Permalink
Don't run include_in_parent when in copy_to context (#87123)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
romseygeek authored Jun 8, 2022
1 parent 4640c03 commit 4e1f725
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 3 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/87123.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 87123
summary: Don't run `include_in_parent` when in `copy_to` context
area: Mapping
type: bug
issues:
- 87036
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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();
Expand All @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

}
}

0 comments on commit 4e1f725

Please sign in to comment.