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.

Sign up for GitHub

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

Jump to bottom

Remove deprecated useDisMax from MultiMatchQuery #36488

Merged
merged 3 commits into from Dec 13, 2018
Merged

Remove deprecated useDisMax from MultiMatchQuery #36488

merged 3 commits into from Dec 13, 2018

Conversation

cbuescher
Copy link
Member

The getters and setters for useDisMax() have been deprecated since at least 6.0,
also there hasn't been any reference to the query parameter in the
documentation. Removing it from the builder and tests and replacing it with
tieBreaker(1.0f) where necessary.

Remove useDisMax from MultiMatchQuery
7a37f91 
The getters and setters for useDisMax() have been deprecated since at least 6.0,
also there hasn't been any reference to the query parameter in the
documentation. Removing it from the builder and tests and replacing it with
`tieBreaker(1.0f)` where necessary.
@cbuescher cbuescher added >non-issue :Search/Search Search-related issues that do not fall into other categories v7.0.0 labels Dec 11, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

jtibshirani
jtibshirani approved these changes Dec 12, 2018
View reviewed changes
Copy link
Contributor

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

Looks good to me. To be really safe we could deprecate the use_dis_max REST parameter in 6.6? The parameter may still pop up in some places -- for example, it looks like it's being used in JdbcCsvSpecIT.

server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java Outdated
@@ -256,7 +256,9 @@ protected void doWriteTo(StreamOutput out) throws IOException {
out.writeVInt(maxExpansions);
out.writeOptionalString(minimumShouldMatch);
out.writeOptionalString(fuzzyRewrite);
out.writeOptionalBoolean(useDisMax);
if (out.getVersion().before(Version.V_7_0_0)) {
out.writeOptionalBoolean(null); //
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment here would be nice (or we could remove the extra marker //)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, leftover. Will remove.

@cbuescher
Copy link
Member Author

To be really safe we could deprecate the use_dis_max REST parameter in 6.6? The parameter may still pop up in some places -- for example, it looks like it's being used in JdbcCsvSpecIT.

I misses this usage in some sql qa tests. Adding deprecation warnings for the rest usages in 6.x are a good idea but I'd say we can still remove it in 7.0 since at least for MultiMatch this parameter hasn't been documented at last since 5.6 (there was some docs for use in Query string query, but there also has been a note in docs/reference/migration/migrate_6_0/search.asciidoc that its use in query_string has been removed).

@cbuescher
Copy link
Member Author

@elasticmachien test this please

@cbuescher cbuescher merged commit b33ff16 into elastic:master Dec 13, 2018
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Dec 13, 2018
@jasontedor
Merge remote-tracking branch 'elastic/master' into ccr-logstash-template
b7a26c7 
* elastic/master:
  Remove deprecated `useDisMax` from MultiMatchQuery (elastic#36488)
  HLRC: Add get users action (elastic#36332)
  fix MultiValuesSourceFieldConfig toXContent (elastic#36525)
  Add ILM-specific security privileges (elastic#36493)
  Remove usages of `MockTcpTransport` from zen tests (elastic#36579)
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this pull request Dec 13, 2018
Warn for use_dis_max in multi_match
2a33afe 
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 mentioned this pull request Dec 13, 2018
Merged
cbuescher pushed a commit that referenced this pull request Dec 13, 2018
Warn for use_dis_max in multi_match (#36614)
5ad71ed 
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 #36488
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtibshirani jtibshirani jtibshirani approved these changes

Assignees
No one assigned
Labels
>non-issue :Search/Search Search-related issues that do not fall into other categories v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@cbuescher @elasticmachine @jtibshirani @colings86