Split RuntimeFieldType from corresponding MappedFieldType #70695
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
So far the runtime section supports only leaf field types, hence the internal representation is based on `RuntimeFieldType` that extends directly `MappedFieldType`. This is straightforward but it is limiting for e.g. an alias field that points to another field, or for object fields that are not queryable directly, hence should not be a MappedFieldType, yet their subfields do. This commit makes `RuntimeFieldType` an interface, effectively splitting the definition of a runtime fields as defined and returned in the mappings, from its internal representation in terms of `MappedFieldType`. The existing runtime script field types still extend `MappedFieldType` and now also implements the new interface, which makes the change rather simple.
javanna
added
:Search Foundations/Mapping
Collaborator
|
Pinging @elastic/es-search (Team:Search) |
javanna
commented
Mar 22, 2021
| * A Builder for a ParametrizedFieldMapper | ||
| */ | ||
| public abstract static class Builder extends Mapper.Builder { | ||
| public abstract static class Builder extends Mapper.Builder implements ToXContentFragment { |
Member
Author
There was a problem hiding this comment.
this is not extremely important, at the same time I got frustrated that we were carrying around a CheckedBiConsumer while we have a commonly used interface for this sort of things. This change allows carrying a ToXContent instance instead in the different script field types
javanna
commented
Mar 22, 2021
| super(name, false, false, false, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta); | ||
| this.toXContent = toXContent; | ||
| } | ||
| public interface RuntimeFieldType extends ToXContentFragment { |
Member
Author
There was a problem hiding this comment.
@romseygeek looks familiar? :) I did take inspiration from your previous proposal around aliases as runtime fields.
Member
Author
There was a problem hiding this comment.
I would propose renaming this to RuntimeField in a follow-up
javanna
commented
Mar 22, 2021
| * Exposes the {@link MappedFieldType} backing this runtime field, used to execute queries, run aggs etc. | ||
| * @return the {@link MappedFieldType} backing this runtime field | ||
| */ | ||
| MappedFieldType asMappedFieldType(); |
Member
Author
There was a problem hiding this comment.
I envision this returning a Collection once we introduce the object runtime field
javanna
commented
Mar 22, 2021
| MappedFieldType fieldType = runtimeFieldType.asMappedFieldType(); | ||
| runtimeFieldTypes.put(fieldType.name(), fieldType); | ||
| } | ||
| return Collections.unmodifiableMap(runtimeFieldTypes); |
Member
Author
There was a problem hiding this comment.
I was thinking that maybe we should make FieldTypeLookup public and use it in SearchExecutionContext instead of reproducing its behavour with the runtime fields defined in the search request
romseygeek
approved these changes
Mar 23, 2021
Contributor
romseygeek
left a comment
romseygeek
left a comment
There was a problem hiding this comment.
What a great idea, who thought of this? :)
LGTM.
javanna
added a commit
to javanna/elasticsearch
that referenced
this pull request
Mar 23, 2021
) So far the runtime section supports only leaf field types, hence the internal representation is based on `RuntimeFieldType` that extends directly `MappedFieldType`. This is straightforward but it is limiting for e.g. an alias field that points to another field, or for object fields that are not queryable directly, hence should not be a MappedFieldType, yet their subfields do. This commit makes `RuntimeFieldType` an interface, effectively splitting the definition of a runtime fields as defined and returned in the mappings, from its internal representation in terms of `MappedFieldType`. The existing runtime script field types still extend `MappedFieldType` and now also implement the new interface, which makes the change rather simple.
javanna
added a commit
that referenced
this pull request
Mar 23, 2021
…70715) So far the runtime section supports only leaf field types, hence the internal representation is based on `RuntimeFieldType` that extends directly `MappedFieldType`. This is straightforward but it is limiting for e.g. an alias field that points to another field, or for object fields that are not queryable directly, hence should not be a MappedFieldType, yet their subfields do. This commit makes `RuntimeFieldType` an interface, effectively splitting the definition of a runtime fields as defined and returned in the mappings, from its internal representation in terms of `MappedFieldType`. The existing runtime script field types still extend `MappedFieldType` and now also implement the new interface, which makes the change rather simple.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So far the runtime section supports only leaf field types, hence the internal representation is based on
RuntimeFieldTypethat extends directlyMappedFieldType. This is straightforward but it is limiting for e.g. an alias field that points to another field, or for object fields that are not queryable directly, hence should not be a MappedFieldType, yet their subfields do.This commit makes
RuntimeFieldTypean interface, effectively splitting the definition of a runtime fields as defined and returned in the mappings, from its internal representation in terms ofMappedFieldType.The existing runtime script field types still extend
MappedFieldTypeand now also implements the new interface, which makes the change rather simple.