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

Require a field when a seed is provided to the random_score function. #25594

Merged
merged 3 commits into from
Jul 19, 2017

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Jul 7, 2017

We currently use fielddata on the _id field which is trappy, especially as we
do it implicitly. This changes the random_score function to use doc ids when
no seed is provided and to suggest a field when a seed is provided.

For now the change only emits a deprecation warning when no field is supplied
but this should be replaced by a strict check on 7.0.

Closes #25240

…ion.

We currently use fielddata on the `_id` field which is trappy, especially as we
do it implicitly. This changes the `random_score` function to use doc ids when
no seed is provided and to suggest a field when a seed is provided.

For now the change only emits a deprecation warning when no field is supplied
but this should be replaced by a strict check on 7.0.

Closes elastic#25240
Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM I left one minor comment on a test


public void testRandomScoreFunctionWithoutSeed() throws Exception {
RandomScoreFunctionBuilder builder = new RandomScoreFunctionBuilder();
builder.seed(42);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test suggests its testing without a seed but then sets one here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, I fixed teh test name

@jpountz jpountz merged commit f1ff7f2 into elastic:master Jul 19, 2017
@jpountz jpountz deleted the fix/random_score branch July 19, 2017 12:11
@clintongormley clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :Query DSL labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories v6.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants