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

Introduce specialized getMatchingFieldTypes that takes a predicate #73618

Conversation

javanna
Copy link
Member

@javanna javanna commented Jun 1, 2021

We would like to reduce usages of FieldTypeLookup#getMatchingFieldTypes. The method is used for two main different reasons:

  1. exists query builder to check whether a field exists or not (see ExistsQueryBuilder to no longer rely on getMatchingFieldTypes #73617)
  2. to iterate through all field types (getMatchingFieldTypes("*") is called internally) and then filter them based on different criteria: this is used to eagerly load global ordinals and find the join field type

The latter use-case could use a more specialized method, which makes it easier to trace. Additionally this scenario does not need to have runtime mappings applied in SearchExecutionContext which simplifies things further as the method can simply proxy to the corresponding MappingLookup#getMatchingFieldTypes.

We would like to reduce usages of FieldTypeLookup#getMatchingFieldTypes. The method is used for two main different reasons:
1) exists query builder to check whether a field exists or not
2) to iterate through all field types (`getMatchingFieldTypes("*")` is called internally) and then filter them based on different criteria: this is used to eagerly load global ordinals and find the join field type

The latter use-case could use a more specialized method, which makes it easier to trace. Additionally this scenario does not need to have runtime mappings applied in `SearchExecutionContext` which simplifies things further as the method can simply proxy to the corresponding `MappingLookup#getMatchingFieldTypes`.
@javanna javanna added :Search Foundations/Mapping Index mappings, including merging and defining field types >refactoring v8.0.0 v7.14.0 labels Jun 1, 2021
@javanna javanna requested review from jtibshirani and jimczi June 1, 2021 14:54
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Jun 1, 2021
@elasticmachine
Copy link
Collaborator

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

for (String knownSubfield : dft.getKnownSubfields()) {
childFieldTypes.add(dft.getChildFieldType(knownSubfield));
}
return childFieldTypes.stream();
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 am thinking that probably, given the current usages of this method, the dynamic field type step can be skipped. getKnownSubfields was just introduced for #73252, and eager global ordinals and join field type (where this method is called) have nothing to do with runtime fields.

@javanna
Copy link
Member Author

javanna commented Jun 2, 2021

Closing in favour of #73655 which takes a different approach by reusing the index time lookup that's already available within MappingLookup.

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.14.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants