Read subobjects mapping parameter in advance #86894
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
As part of elastic#86166 we added support for a new mapping parameter called `subobjects` that can be set to the `object` field type. Such parameter will affect not only how incoming documents will be dinamically mapped in the future, but also how field names are treated as part of the mappings themselves. Mappings get parsed into a map, where keys ordering is not guaranteed. In case a mappings call contains `subobjects` set to `false` and also sub-fields for that same object, we need to make sure that we read the parameter in advance in order to know how to treat the sub-field and decide whether we need to expand dots in field names or leave them as they are.
javanna added >non-issue :Search Foundations/Mapping 
Collaborator
| Pinging @elastic/es-search (Team:Search) |
romseygeek approved these changes May 18, 2022
Contributor
romseygeek left a comment
romseygeek left a comment There was a problem hiding this comment.
One question but LGTM otherwise
| protected static void parseSubobjects(Map<String, Object> node, ObjectMapper.Builder builder) { | ||
| Object subobjectsNode = node.remove("subobjects"); | ||
| if (subobjectsNode != null) { | ||
| builder.subobjects(XContentMapValues.nodeBooleanValue(subobjectsNode, "subobjects.subobjects")); |
Contributor
There was a problem hiding this comment.
subobjects.subobjects looks like a typo?
Member Author
There was a problem hiding this comment.
no it is correct, based on my copy and paste skills
Member Author
There was a problem hiding this comment.
I would have asked the same though in your shoes.
Collaborator
| Pinging @elastic/clients-team (Team:Clients) |
Member Author
| @romseygeek I made subobjects a constructor argument in ObjectMapper.Builder , you may want to have another look when you get a chance. |
romseygeek approved these changes May 23, 2022
Contributor
romseygeek left a comment
romseygeek left a comment There was a problem hiding this comment.
LGTM, thanks @javanna
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
As part of #86166 we added support for a new mapping parameter called
subobjectsthatcan be set to the
objectfield type. Such parameter will affect not only how incomingdocuments will be dynamically mapped in the future, but also how field names are treated
as part of the mappings themselves.
Mappings get parsed into a map, where keys ordering is not guaranteed. In case a mappings call
contains
subobjectsset tofalseand also sub-fields for that same object, we need to makesure that we read the parameter in advance in order to know how to treat the sub-field and decide
whether we need to expand dots in field names or leave them as they are.