Add _index and _version metatada fields to fields api
#79042
Conversation
cbuescher
added
>enhancement
:Search/Search
|
Pinging @elastic/es-search (Team:Search) |
| } | ||
|
|
||
| @Override | ||
| public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier<SearchLookup> searchLookup) { |
There was a problem hiding this comment.
I had to implement this method so fetching version works, the default implementation in MappedFieldType throws an error. I'm not sure if this has undesired effects elsewhere though.
There was a problem hiding this comment.
This will allow _version to be used in aggregations, scripts, etc. This seems okay to me, but maybe worth checking with the team quickly to see if anyone has concerns?
There was a problem hiding this comment.
Good point, will check
There was a problem hiding this comment.
Didn't get any immediate negative concerns, and we already support "index" or "id" in similar cases so I'll go ahad with this.
|
With this change, there are five field mappers left that still throw an |
| XContentBuilder builder = JsonXContent.contentBuilder().startObject(); | ||
| builder.field("field", "value"); | ||
| builder.endObject(); | ||
| SourceToParse source = new SourceToParse(index, "id", BytesReference.bytes(builder), XContentType.JSON, "", Map.of()); |
There was a problem hiding this comment.
nit: use source() here as well?
There was a problem hiding this comment.
I wanted to randomize the index name since that's the value under test here. All the existing source() methods use test as fixed index name and I didn't want to add another method. Do you think its worth it, or do you think we don't need the randomization?
There was a problem hiding this comment.
OIC, no, randomization is good - maybe add a comment explaining why we're using an explicit SourceToParse call?
|
@elasticmachine run elasticsearch-ci/part-1 |
| @Override | ||
| public ValueFetcher valueFetcher(SearchExecutionContext context, String format) { | ||
| throw new UnsupportedOperationException("Cannot fetch values for internal field [" + name() + "]."); | ||
| return new DocValueFetcher(docValueFormat(format, null), context.getForField(this)); |
There was a problem hiding this comment.
I think we could do something even simpler here since the value is constant. We could return a direct implementation of ValueFetcher that always returns fullyQualifiedIndexName.
| } | ||
|
|
||
| @Override | ||
| public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier<SearchLookup> searchLookup) { |
There was a problem hiding this comment.
This will allow _version to be used in aggregations, scripts, etc. This seems okay to me, but maybe worth checking with the team quickly to see if anyone has concerns?
) 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 _index and _version metadata fields which are internally stored as doc values to the list of fields that can be explicitely retrieved. Relates to elastic#75836
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
5 participants
Currently we don't allow retrieving metadata fields through the fields option in search but throw
an error on this case. In #78828 we started to enable this for "_id" if the field is explicitely requested.
This PR adds
_indexand_versionmetadata fields which are internally stored as doc values tothe list of fields that can be explicitely retrieved.
Relates to #75836