Skip to content
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

Merged
merged 14 commits into from
May 23, 2022

Conversation

javanna
Copy link
Member

@javanna javanna commented May 18, 2022

As part of #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 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 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.

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 javanna added >non-issue :Search Foundations/Mapping Index mappings, including merging and defining field types v8.3.0 labels May 18, 2022
@javanna javanna requested a review from romseygeek May 18, 2022 14:41
@elasticmachine elasticmachine added the Team:Search Meta label for search team label May 18, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

Copy link
Contributor

@romseygeek romseygeek left a 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"));
Copy link
Contributor

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?

Copy link
Member Author

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

Copy link
Member Author

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.

@sethmlarson sethmlarson added the Team:Clients Meta label for clients team label May 18, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/clients-team (Team:Clients)

@javanna
Copy link
Member Author

javanna commented May 20, 2022

@romseygeek I made subobjects a constructor argument in ObjectMapper.Builder , you may want to have another look when you get a chance.

Copy link
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @javanna

@javanna javanna merged commit bff9113 into elastic:master May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Clients Meta label for clients team Team:Search Meta label for search team v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants