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

GET operations should not extract fields from _source. #20158

Merged

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Aug 25, 2016

This makes GET operations more consistent with _search operations which expect
(stored_)fields to work on stored fields and source filtering to work on the
_source field. This is now possible thanks to the fact that GET operations
do not read from the translog anymore (#20102) and also allows to get rid of
FieldMapper#isGenerated.

The _termvectors API (and thus more_like_this too) was relying on the fact
that GET operations would extract fields from either stored fields or the source
so the logic to do this that used to exist in ShardGetService has been moved
to TermVectorsService. It would be nice that term vectors do not rely on this,
but this does not seem to be a low hanging fruit.

Relates to #15017 which did the same for the _search API.

@jpountz jpountz added >bug :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v5.0.0-beta1 labels Aug 25, 2016
@jimczi
Copy link
Contributor

jimczi commented Aug 25, 2016

LGTM
Although we need to change the documentation for the get request and add a note to the breaking change.
I can do that in the PR I am working on to solve the inconsistency of our API when fields is used as a parameter.

@jpountz
Copy link
Contributor Author

jpountz commented Aug 25, 2016

That would be awesome. :)

This makes GET operations more consistent with `_search` operations which expect
`(stored_)fields` to work on stored fields and source filtering to work on the
`_source` field. This is now possible thanks to the fact that GET operations
do not read from the translog anymore (elastic#20102) and also allows to get rid of
`FieldMapper#isGenerated`.

The `_termvectors` API (and thus more_like_this too) was relying on the fact
that GET operations would extract fields from either stored fields or the source
so the logic to do this that used to exist in `ShardGetService` has been moved
to `TermVectorsService`. It would be nice that term vectors do not rely on this,
but this does not seem to be a low hanging fruit.
@jpountz jpountz force-pushed the fix/get_should_not_extract_fields_from_source branch from 7b44cb0 to 3ed0da5 Compare August 26, 2016 08:35
@jpountz jpountz merged commit 3ed0da5 into elastic:master Aug 26, 2016
@jpountz jpountz deleted the fix/get_should_not_extract_fields_from_source branch August 26, 2016 08:36
elasticsearchmachine pushed a commit that referenced this pull request Apr 20, 2022
668d557
added some `todo` in `ShardGetService` suggest that source might parsed
and available in the sourceLookup.
https://github.com/elastic/elasticsearch/blob/668d55758b751e848f2b82299e2c5e74af55cd12/src/main/java/org/elasticsearch/index/get/ShardGetService.java#L315
https://github.com/elastic/elasticsearch/blob/668d55758b751e848f2b82299e2c5e74af55cd12/src/main/java/org/elasticsearch/index/get/ShardGetService.java#L431
However ,`SourceLookup` in `ShardGetService` has been remove in #20158
which made these `todo` invalid.  One of them has been removed in #83152
This PR remove the left invalid `todo` in `ShardGetService`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v5.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants