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

Warn when using use_dis_max in multi_match #36614

Merged
merged 2 commits into from Dec 13, 2018
Merged

Warn when using use_dis_max in multi_match #36614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a33afe
Warn for `use_dis_max` in `multi_match`
Dec 13, 2018
79fe21c
Merge branch '6.x' into multimatch-disMax-deprecation-rest
Dec 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java
View file
Open in desktop
Expand Up @@ -67,7 +67,7 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder<MultiMatchQuery
private static final ParseField LENIENT_FIELD = new ParseField("lenient");
private static final ParseField CUTOFF_FREQUENCY_FIELD = new ParseField("cutoff_frequency");
private static final ParseField TIE_BREAKER_FIELD = new ParseField("tie_breaker");
private static final ParseField USE_DIS_MAX_FIELD = new ParseField("use_dis_max");
private static final ParseField USE_DIS_MAX_FIELD = new ParseField("use_dis_max").withAllDeprecated("use tie_breaker instead");
private static final ParseField FUZZY_REWRITE_FIELD = new ParseField("fuzzy_rewrite");
private static final ParseField MINIMUM_SHOULD_MATCH_FIELD = new ParseField("minimum_should_match");
private static final ParseField OPERATOR_FIELD = new ParseField("operator");
Expand Down
16 changes: 13 additions & 3 deletions server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java
View file
Open in desktop
Expand Up @@ -124,9 +124,6 @@ protected MultiMatchQueryBuilder doCreateTestQueryBuilder() {
if (randomBoolean()) {
query.fuzzyRewrite(getRandomRewriteMethod());
}
if (randomBoolean()) {
query.useDisMax(randomBoolean());
}
if (randomBoolean()) {
query.tieBreaker(randomFloat());
}
Expand Down Expand Up @@ -476,6 +473,19 @@ public void testWithStopWords() throws Exception {
assertEquals(expected, query);
}

public void testDisMaxDeprecation() throws Exception {
String json =
"{\n" +
" \"multi_match\" : {\n" +
" \"query\" : \"foo:bar\",\n" +
" \"use_dis_max\" : true\n" +
" }\n" +
"}";

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).

}

private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings, Settings indexSettings) {
Settings build = Settings.builder().put(oldIndexSettings)
.put(indexSettings)
Expand Down