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

Pass a SearchLookup supplier through to fielddataBuilder #60224

Conversation

javanna
Copy link
Member

@javanna javanna commented Jul 27, 2020

Runtime fields depend on SearchLookup to be able to look up other fields and manipulate them. With this change we add a SearchLookup argument to the existing MappedFieldType#fielddataBuilder method. Additionally the use of runtime fields in index sorting is disallowed.

This is essentially #58527 but refined and made to the runtime fields feature branch. We have introduced a temporary way to make SearchLookup available as part of #58939 to minimize merge conflicts in a feature branch, but it is now time to make this change permanent.

Relates to #59332

Runtime fields depend on SearchLookup to be able to look up other fields and manipulate them. With this change we add a SearchLookup argument to the existing MappedFieldType#fielddataBuilder method. Additionally the use of runtime fields in index sorting is disallowed.

Relates to elastic#59332
@javanna javanna added the :Search/Search Search-related issues that do not fall into other categories label Jul 27, 2020
@javanna javanna requested a review from nik9000 July 27, 2020 15:32
@elasticmachine
Copy link
Collaborator

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

@elasticmachine elasticmachine added the Team:Search Meta label for search team label Jul 27, 2020
@javanna javanna mentioned this pull request Jul 27, 2020
30 tasks
Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

Great!

@@ -26,26 +24,37 @@

protected abstract String runtimeType();

@SuppressWarnings("unused")
Copy link
Member

Choose a reason for hiding this comment

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

Is this because intellij doesn't like these methods because they are only called by the test framework?

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, not that important, I got annoyed with all the warnings hence I addressed them. I can also revert if there are cons

Copy link
Member

Choose a reason for hiding this comment

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

Nah. No need. I didn't realize IntelliJ would warn about it.

@javanna javanna merged commit 9026770 into elastic:feature/runtime_fields Jul 27, 2020
javanna added a commit to javanna/elasticsearch that referenced this pull request Aug 20, 2020
Runtime fields look up other fields through `SearchLookup`. With this change we add a `Supplier<SearchLookup>` argument to the existing `MappedFieldType#fielddataBuilder` method so that fielddata implementations can have `SearchLookup` available when needed.

Note that this commit does not introduce any production implementation of runtime fields, but rather the plumbing needed to then merge the feature branch where runtime fields have been developed.

Additionally the use of runtime fields in index sorting is disallowed.

Relates to elastic#59332
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants