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

Warn when using use_dis_max in multi_match #36614

Merged
merged 2 commits into from Dec 13, 2018

Conversation

cbuescher
Copy link
Member

The rest parameter is already deprecated at least since 6.0, this PR only adds
an additional warning when the parameter gets used via the query DSL.

Relates to #36488

The rest parameter is already deprecated at least since 6.0, we should also add
a warnign when the parameter gets used via the query DSL.

Relates to elastic#36488
@cbuescher cbuescher added :Search/Search Search-related issues that do not fall into other categories v6.6.0 labels Dec 13, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@cbuescher
Copy link
Member Author

As a follow up to #36488

"}";

parseQuery(json);
assertWarnings("Deprecated field [use_dis_max] used, replaced by [use tie_breaker instead]");
Copy link
Contributor

Choose a reason for hiding this comment

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

The syntax [use tie_breaker instead] is a little confusing because square brackets usually contain single names, should it just be [tie_breaker] or should we emit a fully custom message?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree it looks slightly weird but thats what we usuall have done with ParseField deprecations in the past. "tieBreaker" is not a boolean flag so its not a direct replacements, also "tieBreaker" would otherwise act as a deprecated alias for "use_dis_max" which it isn't.
See e.g. QueryStringQueryBuilderL108 (at least on 6.x) for similar use cases where the message will contain brackets as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I now see that it's consistent with our other messages (which seems most important).

@cbuescher cbuescher changed the title Warn for use_dis_max in multi_match Warn when using use_dis_max in multi_match Dec 13, 2018
@cbuescher cbuescher merged commit 5ad71ed into elastic:6.x Dec 13, 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.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants