-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Support fetching flattened subfields #70916
Support fetching flattened subfields #70916
Conversation
Currently the `fields` API fetches the root flattened field and returns it in a structured way in the response. In addition this change makes it possible to directly query subfields. However, requesting flattened subfields via wildcard patterns is not possible. Closes elastic#70605
Pinging @elastic/es-search (Team:Search) |
server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is generally the right way to go - left a couple of comments.
server/src/main/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapper.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, just left some small suggestions.
server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapper.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/index/mapper/flattened/KeyedFlattenedFieldTypeTests.java
Show resolved
Hide resolved
@jtibshirani thanks, I think I adressed your change requests and added a unit and a yaml test. I think I will also add a short passage to the docs somewhere about fetching flattened subfields and that the naming has to be exact, I haven't found a good spot yet though. Will be back with another short update shortly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me! I added some last minor comments.
@@ -325,6 +325,77 @@ will return only the users first name but still maintain the structure of the ne | |||
However, when the `fields` pattern targets the nested `user` field directly, no | |||
values will be returned since the pattern doesn't match any leaf fields. | |||
|
|||
[discrete] | |||
[[search-fields-flattened]] | |||
==== Handling of flattened fields |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to move this to the flattened
field type documentation. That would help keep this high-level documentation streamlined and easy to approach.
@@ -261,8 +263,7 @@ public BytesRef indexedValueForSearch(Object value) { | |||
|
|||
@Override | |||
public ValueFetcher valueFetcher(SearchExecutionContext context, String format) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized we should throw an error if the format is non-null, as we do for most other field types. (This is unfortunately very manual right now and easy to forget...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the iterations @cbuescher
Currently the `fields` API fetches the root flattened field and returns it in a structured way in the response. In addition this change makes it possible to directly query subfields. However, requesting flattened subfields via wildcard patterns is not possible. Closes elastic#70605
Currently the `fields` API fetches the root flattened field and returns it in a structured way in the response. In addition this change makes it possible to directly query subfields. However, requesting flattened subfields via wildcard patterns is not possible. Closes #70605
Currently the
fields
API fetches the root flattened field and returns it in astructured way in the response. In addition this change makes it possible to
directly query subfields. However, requesting flattened subfields via wildcard
patterns is not possible.
Closes #70605