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

Remove payload option from completion suggester #19877

Merged
merged 1 commit into from
Aug 9, 2016

Conversation

areek
Copy link
Contributor

@areek areek commented Aug 8, 2016

The payload option was introduced with the new completion
suggester implementation in v5, as a stop gap solution
to return additional metadata with suggestions.

Now we can return associated documents with suggestions
(#19536) through an additional fetch phase using stored
field (_source). The fetch phase ensures that we only fetch
the _source for the global top-N suggestions instead of
fetching _source of top results for each shard.

@areek areek added >enhancement review :Search Relevance/Suggesters "Did you mean" and suggestions as you type v5.0.0-beta1 labels Aug 8, 2016
The payload option was introduced with the new completion
suggester implementation in v5, as a stop gap solution
to return additional metadata with suggestions.

Now we can return associated documents with suggestions
(elastic#19536) through fetch phase using stored field (_source).
The additional fetch phase ensures that we only fetch
the _source for the global top-N suggestions instead of
fetching _source of top results for each shard.
@areek areek force-pushed the fix/remove_completion_payload branch from 3db40c0 to d107141 Compare August 8, 2016 20:04
@jimczi
Copy link
Contributor

jimczi commented Aug 9, 2016

LGTM for the removal.
I am concerned about the fetch phase though. It seems that we always retrieve the _id of the suggestion even if _source is disabled. This is expected since it is now the same code that retrieves the stored fields for the search and the suggestion. For the top docs the assumption is that the _id is always needed so we have to visit all the stored fields but for suggestions I am not sure this is what we want ?

@nik9000
Copy link
Member

nik9000 commented Aug 9, 2016

For the top docs the assumption is that the _id is always needed

I wish we could turn that off too.... It'd be nice not to have to go to stored fields at all sometimes. If you only want doc_values fields it is lame you still need to go get _id from stored fields.

@areek
Copy link
Contributor Author

areek commented Aug 9, 2016

Thanks for the review and suggestion ;) @jimferenczi and @nik9000! I agree it would be nice if we could turn off retrieving the _id field for suggestion when _source is disabled. I will open a subsequent PR for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search Relevance/Suggesters "Did you mean" and suggestions as you type v5.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants