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

FieldTypeLookup to not implement Iterable #64983

Merged

Conversation

javanna
Copy link
Member

@javanna javanna commented Nov 12, 2020

FieldTypeLookup implements Iterable which is currently only used in IndexWarmer to iterate through all field types that have eager global ordinals set to true. Implementing Iterable makes it hard to track who iterates through all the field types.

This commit exposes a new method that allows to provide a Predicate and returns an Iterable of all the field types that match the predicate.

FieldTypeLookup implements Iterable which is currently only used in IndexWarmer to iterate through all field types that have eager global ordinals set to true. Implementing Iterable makes it hard to track who iterates through all the field types.

This commit exposes a new method that allows to provide a Predicate and returns an Iterable of all the field types that match the predicate.
@javanna javanna added :Search Foundations/Mapping Index mappings, including merging and defining field types >refactoring v8.0.0 v7.11.0 labels Nov 12, 2020
@elasticmachine
Copy link
Collaborator

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

@elasticmachine elasticmachine added the Team:Search Meta label for search team label Nov 12, 2020
public Iterable<MappedFieldType> fieldTypes() {
return this.mapper == null ? Collections.emptySet() : this.mapper.mappers().fieldTypes();
public Iterable<MappedFieldType> fieldTypes(Predicate<MappedFieldType> predicate) {
return this.mapper == null ? Collections.emptySet() : this.mapper.mappers().fieldTypes().filter(predicate);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that we only use this for index warmers, maybe we should replace this with a getEagerGlobalOrdinalsFields() method that returns a set of field names, and which calls FieldTypeLookup.filter() under the hood?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes I agree. will do that

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

@javanna javanna merged commit 6b62d81 into elastic:master Nov 12, 2020
javanna added a commit to javanna/elasticsearch that referenced this pull request Nov 12, 2020
FieldTypeLookup implements Iterable which is currently only used in IndexWarmer to iterate through all field types that have eager global ordinals set to true. Implementing Iterable makes it hard to track who iterates through all the field types.

This commit exposes a new method that allows to provide a Predicate and returns an Iterable of all the field types that match the predicate. Also, MapperService no longer exposes a way to iterate through all the field types, but rather only a specific method to iterate through the field types that have eager global ordinals.
javanna added a commit that referenced this pull request Nov 12, 2020
FieldTypeLookup implements Iterable which is currently only used in IndexWarmer to iterate through all field types that have eager global ordinals set to true. Implementing Iterable makes it hard to track who iterates through all the field types.

This commit exposes a new method that allows to provide a Predicate and returns an Iterable of all the field types that match the predicate. Also, MapperService no longer exposes a way to iterate through all the field types, but rather only a specific method to iterate through the field types that have eager global ordinals.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>refactoring :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team v7.11.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants