-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Read subobjects mapping parameter in advance #86894
Conversation
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.
Pinging @elastic/es-search (Team:Search) |
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.
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")); |
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.
subobjects.subobjects
looks like a typo?
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.
no it is correct, based on my copy and paste skills
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 would have asked the same though in your shoes.
Pinging @elastic/clients-team (Team:Clients) |
@romseygeek I made subobjects a constructor argument in ObjectMapper.Builder , you may want to have another look when you get a chance. |
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.
LGTM, thanks @javanna
As part of #86166 we added support for a new mapping parameter called
subobjects
thatcan be set to the
object
field 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
subobjects
set tofalse
and 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.