Add _ignored and _routing metatada fields to fields api
#78981
Conversation
cbuescher
added
>enhancement
:Search/Search
|
Pinging @elastic/es-search (Team:Search) |
|
fyi I also briefly looked into adding "_source" the same way since it is also kept as a stored field (see |
|
|
||
| - length: { hits.hits.0.fields : 1 } | ||
| - match: { hits.hits.0.fields._id.0: "1" } | ||
| - length: { hits.hits.1.fields : 1 } |
There was a problem hiding this comment.
This yml test has gotten really large! I find it harder to debug yml tests than unit tests -- would it be possible to add these tests in FieldFetcherTests instead?
There was a problem hiding this comment.
I can look into that. I already started doing this for "_id" but found it required some amount of mocking etc... because for the metadata fields I think we need to really index documents somewhere, even if it is a mocked out lucene index. But I might be wrong there. I share your opinion that yml tests are harder too debug, thats why I added some basic unit tests to the mapper tests here. I think these additional tests are not that large but I understand the point.
There was a problem hiding this comment.
One comment though: yml tests are the ones we run against mixed clusters, in upgrade tests etc. so to cover some basic functionality like fetching metadata via "fields" might be good to have anyway, even if I add tests to FieldFetcherTests, wdyt?
There was a problem hiding this comment.
My perspective is it's great to have high-level coverage in yml tests (for example we already have a test that we can load an example metadata field _id). But for more comprehensive tests (like where we check each field type individually), these make sense as unit tests.
|
@jtibshirani thanks for the first review, I extended the existing |
jtibshirani
left a comment
jtibshirani
left a comment
There was a problem hiding this comment.
This looks good to me. I don't find the mocking in the test to be excessive, and the test case looks easy to build on.
|
@jtibshirani thanks for the review |
…78981) Currently we don't allow retrieving metadata fields through the fields option in search but throw an error on this case. In elastic#78828 we started to enable this for `_id` if the field is explicitely requested. This PR adds `_ignored` and `_routing` metadata fields which are also internally handled as stored fields to the list of fields that can be explicitely retrieved.
Currently we exclude metadata fields from being looked up using the fields option in search. However, as issue like #75836 show, they can still be retrieved e.g. via aliases and then fetching their values causes errors. With this change, we enable retrieval of metadata fields (like `_id`, `_ignored` etc.) using the fields option when the field is explicitely requested. We still continue to exclude any metadata field from matching wildcard patterns, but they should be retrievable via an exact name or if there is an alias definition with a path to a metadata field. This change adds support for the `_id`, `_routing`, `_ignored`, `_index` and `_version` field in particular. Backport of #78828, #78981 and #79042
4 participants
Currently we don't allow retrieving metadata fields through the
fieldsoption in search but throwan error on this case. In #78828 we started to enable this for "_id" if the field is explicitely requested.
This PR adds
_ignoredand_routingmetadata fields which are also internally handled as stored fields tothe list of fields that can be explicitely retrieved.
Relates to #75836