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

Make sure to apply eager_global_ordinals for keyed JSON fields. #41319

Merged

Conversation

jtibshirani
Copy link
Contributor

@jtibshirani jtibshirani commented Apr 17, 2019

The index warmer iterates through all field types when determining the fields
for which global ordinals should be loaded. Previously, keyed JSON field types
were not returned from FieldTypeLookup#iterator, so their eager_global_ordinals
setting would be ignored. This PR fixes the issue by including keyed JSON fields
in FieldTypeLookup#iterator.

The index warmer iterates through all field types when determining the fields
for which global ordinals should be loaded. Previously, keyed JSON field types
were not returned from FieldTypeLookup#iterator, so their eager_global_ordinals
setting would be ignored. This PR fixes the issue by including keyed JSON fields
in FieldTypeLookup#iterator.
@jtibshirani jtibshirani added >bug :Search Foundations/Mapping Index mappings, including merging and defining field types labels Apr 17, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@jtibshirani jtibshirani changed the title Make sure to return keyed JSON field types in FieldTypeLookup#iterator. Ensure eager_global_ordinals is respected for keyed JSON fields. Apr 17, 2019
@jtibshirani jtibshirani changed the title Ensure eager_global_ordinals is respected for keyed JSON fields. Make sure to apply eager_global_ordinals for keyed JSON fields. Apr 17, 2019
@jtibshirani jtibshirani merged commit 9f315e7 into elastic:object-fields Apr 18, 2019
@jtibshirani jtibshirani deleted the eager-global-ordinals branch April 18, 2019 16:47
jtibshirani added a commit that referenced this pull request May 1, 2019
…r. (#41319)

The index warmer iterates through all field types when determining the fields
for which global ordinals should be loaded. Previously, keyed JSON field types
were not returned from FieldTypeLookup#iterator, so their eager_global_ordinals
setting would be ignored. This PR fixes the issue by including keyed JSON fields
in FieldTypeLookup#iterator.
jtibshirani added a commit that referenced this pull request May 24, 2019
…r. (#41319)

The index warmer iterates through all field types when determining the fields
for which global ordinals should be loaded. Previously, keyed JSON field types
were not returned from FieldTypeLookup#iterator, so their eager_global_ordinals
setting would be ignored. This PR fixes the issue by including keyed JSON fields
in FieldTypeLookup#iterator.
if (fullNameToJsonMapper.isEmpty()) {
return concreteFieldTypes;
} else {
Iterator<MappedFieldType> keyedJsonFieldTypes = fullNameToJsonMapper.values().stream()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A note for future reference: the other option would have been to expose the field type by adding a 'keyed' field mapper in JsonFieldMapper#iterator. I didn't go down that path because this would make my_json_field._keyed a valid field to search on, which we want to avoid.

jtibshirani added a commit that referenced this pull request May 29, 2019
…r. (#41319)

The index warmer iterates through all field types when determining the fields
for which global ordinals should be loaded. Previously, keyed JSON field types
were not returned from FieldTypeLookup#iterator, so their eager_global_ordinals
setting would be ignored. This PR fixes the issue by including keyed JSON fields
in FieldTypeLookup#iterator.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search Foundations/Mapping Index mappings, including merging and defining field types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants