Track synthetic source for disabled objects#108051
Conversation
|
Hi @kkrik-es, I've created a changelog YAML for you. |
…nto fix/synthetic-source/disabled
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
martijnvg
left a comment
There was a problem hiding this comment.
Looks good. Left two questions.
| ) | ||
| ); | ||
| } | ||
| context.parser().skipChildren(); |
There was a problem hiding this comment.
should context.parser().skipChildren() only be executed if context.isSourceSynthetic() == false? If so then add an else clause here?
There was a problem hiding this comment.
I think it's fine as is, the _ignored_source entry created above includes all children so there's no point to further inspect this object. The tests support that this is the case?
There was a problem hiding this comment.
I see, this is a no-op for the new path.
| ) | ||
| ); | ||
| } | ||
| parser.skipChildren(); |
server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java
Outdated
Show resolved
Hide resolved
| assertThat(syntheticSource, Matchers.containsString("\"some\":{\"deeply\":{\"nested\":{\"string_value\":\"" + stringValue + "\"")); | ||
| } | ||
|
|
||
| public void testDisabledObjectSingleField() throws IOException { |
There was a problem hiding this comment.
Maybe add a test with many object fields at the same level (some enabled and some disabled)? I think this should catch the case were the previous skipChildren(...) invocation after XContentDataHelper.encodeToken(...) could be an issue.
|
Does this also track the source of objects where dynamic mapping is disabled ( |
No, but I'll take a look at that too. |
Tracks source for disabled object in
_ignored_sourcemetadata field.Related to #106825