-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Shadowed mappers need to skip their children #65811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Shadowed mappers need to skip their children #65811
Conversation
|
Pinging @elastic/es-search (Team:Search) |
|
Marking as a |
javanna
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, left a couple of comments
| return Collections.singleton(new RuntimeFields()); | ||
| } | ||
|
|
||
| public void testRuntimeFieldShadowsObject() throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this test fit into the existing DocumentParserTests? I have added similar tests in there as part of #65210
| protected void parseCreateField(ParseContext context) throws IOException { | ||
| //field defined as runtime field, don't index anything | ||
| //field defined as runtime field, don't index anything but skip over any possible children | ||
| context.parser().skipChildren(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not 100% sure that this is the expected behaviour. The way they are applied, runtime fields are an additional layer on top of ordinary fields. because an object field is shadowed by a runtime field, we end up skipping all of the children structure and not index fields that would not be directly shadowed. If doable, could we consider instead mapping objects as objects even if they are shadowed? That would be more inline with the statement that "objects cannot be defined in the runtime section".
| type: date | ||
| location: | ||
| type: object | ||
| enabled: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why here you haven't applied dynamic: false like in other tests?
|
I'm going to close this and open a separate PR to deal with object children, given that the actual solution is sufficiently different from what I have here. |
When a runtime field is defined in mappings, it automatically shadows any
corresponding fields that might be dynamically created. If a document is
added with an object that matches a runtime field, we need to make sure
that all of the object's children are skipped during parsing.