Skip to content

Commit

Permalink
Remove deprecated useDisMax from MultiMatchQuery (#36488)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Christoph Büscher committed Dec 13, 2018
1 parent 4b17055 commit b33ff16
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 63 deletions.
Expand Up @@ -67,7 +67,6 @@ 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 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 All @@ -92,7 +91,6 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder<MultiMatchQuery
private int maxExpansions = DEFAULT_MAX_EXPANSIONS;
private String minimumShouldMatch;
private String fuzzyRewrite = null;
private Boolean useDisMax;
private Float tieBreaker;
private Boolean lenient;
private Float cutoffFrequency = null;
Expand Down Expand Up @@ -224,7 +222,9 @@ public MultiMatchQueryBuilder(StreamInput in) throws IOException {
maxExpansions = in.readVInt();
minimumShouldMatch = in.readOptionalString();
fuzzyRewrite = in.readOptionalString();
useDisMax = in.readOptionalBoolean();
if (in.getVersion().before(Version.V_7_0_0)) {
in.readOptionalBoolean(); // unused use_dis_max flag
}
tieBreaker = in.readOptionalFloat();
if (in.getVersion().onOrAfter(Version.V_6_1_0)) {
lenient = in.readOptionalBoolean();
Expand Down Expand Up @@ -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);
}
out.writeOptionalFloat(tieBreaker);
if (out.getVersion().onOrAfter(Version.V_6_1_0)) {
out.writeOptionalBoolean(lenient);
Expand Down Expand Up @@ -438,20 +440,6 @@ public String fuzzyRewrite() {
return fuzzyRewrite;
}

/**
* @deprecated use a tieBreaker of 1.0f to disable "dis-max"
* query or select the appropriate {@link Type}
*/
@Deprecated
public MultiMatchQueryBuilder useDisMax(Boolean useDisMax) {
this.useDisMax = useDisMax;
return this;
}

public Boolean useDisMax() {
return useDisMax;
}

/**
* <p>Tie-Breaker for "best-match" disjunction queries (OR-Queries).
* The tie breaker capability allows documents that match more than one query clause
Expand Down Expand Up @@ -593,9 +581,6 @@ public void doXContent(XContentBuilder builder, Params params) throws IOExceptio
if (fuzzyRewrite != null) {
builder.field(FUZZY_REWRITE_FIELD.getPreferredName(), fuzzyRewrite);
}
if (useDisMax != null) {
builder.field(USE_DIS_MAX_FIELD.getPreferredName(), useDisMax);
}
if (tieBreaker != null) {
builder.field(TIE_BREAKER_FIELD.getPreferredName(), tieBreaker);
}
Expand Down Expand Up @@ -674,8 +659,6 @@ public static MultiMatchQueryBuilder fromXContent(XContentParser parser) throws
minimumShouldMatch = parser.textOrNull();
} else if (FUZZY_REWRITE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
fuzzyRewrite = parser.textOrNull();
} else if (USE_DIS_MAX_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
useDisMax = parser.booleanValue();
} else if (TIE_BREAKER_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
tieBreaker = parser.floatValue();
} else if (CUTOFF_FREQUENCY_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
Expand Down Expand Up @@ -724,7 +707,6 @@ public static MultiMatchQueryBuilder fromXContent(XContentParser parser) throws
.cutoffFrequency(cutoffFrequency)
.fuzziness(fuzziness)
.fuzzyRewrite(fuzzyRewrite)
.useDisMax(useDisMax)
.maxExpansions(maxExpansions)
.minimumShouldMatch(minimumShouldMatch)
.operator(operator)
Expand Down Expand Up @@ -798,16 +780,6 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
multiMatchQuery.setAutoGenerateSynonymsPhraseQuery(autoGenerateSynonymsPhraseQuery);
multiMatchQuery.setTranspositions(fuzzyTranspositions);

if (useDisMax != null) { // backwards foobar
boolean typeUsesDismax = type.tieBreaker() != 1.0f;
if (typeUsesDismax != useDisMax) {
if (useDisMax && tieBreaker == null) {
multiMatchQuery.setTieBreaker(0.0f);
} else {
multiMatchQuery.setTieBreaker(1.0f);
}
}
}
Map<String, Float> newFieldsBoosts;
if (fieldsBoosts.isEmpty()) {
// no fields provided, defaults to index.query.default_field
Expand All @@ -828,7 +800,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
@Override
protected int doHashCode() {
return Objects.hash(value, fieldsBoosts, type, operator, analyzer, slop, fuzziness,
prefixLength, maxExpansions, minimumShouldMatch, fuzzyRewrite, useDisMax, tieBreaker, lenient,
prefixLength, maxExpansions, minimumShouldMatch, fuzzyRewrite, tieBreaker, lenient,
cutoffFrequency, zeroTermsQuery, autoGenerateSynonymsPhraseQuery, fuzzyTranspositions);
}

Expand All @@ -845,7 +817,6 @@ protected boolean doEquals(MultiMatchQueryBuilder other) {
Objects.equals(maxExpansions, other.maxExpansions) &&
Objects.equals(minimumShouldMatch, other.minimumShouldMatch) &&
Objects.equals(fuzzyRewrite, other.fuzzyRewrite) &&
Objects.equals(useDisMax, other.useDisMax) &&
Objects.equals(tieBreaker, other.tieBreaker) &&
Objects.equals(lenient, other.lenient) &&
Objects.equals(cutoffFrequency, other.cutoffFrequency) &&
Expand Down
Expand Up @@ -118,9 +118,6 @@ protected MultiMatchQueryBuilder doCreateTestQueryBuilder() {
if (randomBoolean()) {
query.fuzzyRewrite(getRandomRewriteMethod());
}
if (randomBoolean()) {
query.useDisMax(randomBoolean());
}
if (randomBoolean()) {
query.tieBreaker(randomFloat());
}
Expand Down Expand Up @@ -189,7 +186,7 @@ public void testToQueryBoost() throws IOException {
}

public void testToQueryMultipleTermsBooleanQuery() throws Exception {
Query query = multiMatchQuery("test1 test2").field(STRING_FIELD_NAME).useDisMax(false).toQuery(createShardContext());
Query query = multiMatchQuery("test1 test2").field(STRING_FIELD_NAME).toQuery(createShardContext());
assertThat(query, instanceOf(BooleanQuery.class));
BooleanQuery bQuery = (BooleanQuery) query;
assertThat(bQuery.clauses().size(), equalTo(2));
Expand All @@ -198,8 +195,8 @@ public void testToQueryMultipleTermsBooleanQuery() throws Exception {
}

public void testToQueryMultipleFieldsDisableDismax() throws Exception {
Query query = multiMatchQuery("test").field(STRING_FIELD_NAME).field(STRING_FIELD_NAME_2).useDisMax(false)
.toQuery(createShardContext());
Query query = multiMatchQuery("test").field(STRING_FIELD_NAME).field(STRING_FIELD_NAME_2).tieBreaker(1.0f)
.toQuery(createShardContext());
assertThat(query, instanceOf(DisjunctionMaxQuery.class));
DisjunctionMaxQuery dQuery = (DisjunctionMaxQuery) query;
assertThat(dQuery.getTieBreakerMultiplier(), equalTo(1.0f));
Expand All @@ -209,8 +206,7 @@ public void testToQueryMultipleFieldsDisableDismax() throws Exception {
}

public void testToQueryMultipleFieldsDisMaxQuery() throws Exception {
Query query = multiMatchQuery("test").field(STRING_FIELD_NAME).field(STRING_FIELD_NAME_2).useDisMax(true)
.toQuery(createShardContext());
Query query = multiMatchQuery("test").field(STRING_FIELD_NAME).field(STRING_FIELD_NAME_2).toQuery(createShardContext());
assertThat(query, instanceOf(DisjunctionMaxQuery.class));
DisjunctionMaxQuery disMaxQuery = (DisjunctionMaxQuery) query;
assertThat(disMaxQuery.getTieBreakerMultiplier(), equalTo(0.0f));
Expand All @@ -222,7 +218,7 @@ public void testToQueryMultipleFieldsDisMaxQuery() throws Exception {
}

public void testToQueryFieldsWildcard() throws Exception {
Query query = multiMatchQuery("test").field("mapped_str*").useDisMax(false).toQuery(createShardContext());
Query query = multiMatchQuery("test").field("mapped_str*").tieBreaker(1.0f).toQuery(createShardContext());
assertThat(query, instanceOf(DisjunctionMaxQuery.class));
DisjunctionMaxQuery dQuery = (DisjunctionMaxQuery) query;
assertThat(dQuery.getTieBreakerMultiplier(), equalTo(1.0f));
Expand Down
Expand Up @@ -203,7 +203,7 @@ public void testDefaults() throws ExecutionException, InterruptedException {

searchResponse = client().prepareSearch("test")
.setQuery(randomizeType(multiMatchQuery("marvel hero captain america", "full_name", "first_name", "last_name", "category")
.operator(Operator.OR).useDisMax(false).type(type))).get();
.operator(Operator.OR).type(type))).get();
assertFirstHit(searchResponse, anyOf(hasId("theone"), hasId("theother")));
assertThat(searchResponse.getHits().getHits()[0].getScore(), greaterThan(searchResponse.getHits().getHits()[1].getScore()));

Expand Down Expand Up @@ -323,14 +323,14 @@ public void testCutoffFreq() throws ExecutionException, InterruptedException {
cutoffFrequency = randomBoolean() ? Math.min(1, numDocs * 1.f / between(10, 20)) : 1.f / between(10, 20);
searchResponse = client().prepareSearch("test")
.setQuery(randomizeType(multiMatchQuery("marvel hero captain america", "full_name", "first_name", "last_name", "category")
.operator(Operator.OR).useDisMax(false).cutoffFrequency(cutoffFrequency).type(type))).get();
.operator(Operator.OR).cutoffFrequency(cutoffFrequency).type(type))).get();
assertFirstHit(searchResponse, anyOf(hasId("theone"), hasId("theother")));
assertThat(searchResponse.getHits().getHits()[0].getScore(), greaterThan(searchResponse.getHits().getHits()[1].getScore()));
long size = searchResponse.getHits().getTotalHits().value;

searchResponse = client().prepareSearch("test")
.setQuery(randomizeType(multiMatchQuery("marvel hero captain america", "full_name", "first_name", "last_name", "category")
.operator(Operator.OR).useDisMax(false).type(type))).get();
.operator(Operator.OR).type(type))).get();
assertFirstHit(searchResponse, anyOf(hasId("theone"), hasId("theother")));
assertThat("common terms expected to be a way smaller result set", size, lessThan(searchResponse.getHits().getTotalHits().value));

Expand Down Expand Up @@ -399,7 +399,7 @@ public void testEquivalence() {
SearchResponse left = client().prepareSearch("test").setSize(numDocs)
.addSort(SortBuilders.scoreSort()).addSort(SortBuilders.fieldSort("_id"))
.setQuery(randomizeType(multiMatchQueryBuilder
.operator(op).useDisMax(false).minimumShouldMatch(minShouldMatch).type(type))).get();
.operator(op).tieBreaker(1.0f).minimumShouldMatch(minShouldMatch).type(type))).get();

SearchResponse right = client().prepareSearch("test").setSize(numDocs)
.addSort(SortBuilders.scoreSort()).addSort(SortBuilders.fieldSort("_id"))
Expand All @@ -418,7 +418,7 @@ public void testEquivalence() {
SearchResponse left = client().prepareSearch("test").setSize(numDocs)
.addSort(SortBuilders.scoreSort()).addSort(SortBuilders.fieldSort("_id"))
.setQuery(randomizeType(multiMatchQuery("capta", "full_name", "first_name", "last_name", "category")
.type(MatchQuery.Type.PHRASE_PREFIX).useDisMax(false).minimumShouldMatch(minShouldMatch))).get();
.type(MatchQuery.Type.PHRASE_PREFIX).tieBreaker(1.0f).minimumShouldMatch(minShouldMatch))).get();

SearchResponse right = client().prepareSearch("test").setSize(numDocs)
.addSort(SortBuilders.scoreSort()).addSort(SortBuilders.fieldSort("_id"))
Expand All @@ -437,7 +437,7 @@ public void testEquivalence() {
left = client().prepareSearch("test").setSize(numDocs)
.addSort(SortBuilders.scoreSort()).addSort(SortBuilders.fieldSort("_id"))
.setQuery(randomizeType(multiMatchQuery("captain america", "full_name", "first_name", "last_name", "category")
.type(MatchQuery.Type.PHRASE).useDisMax(false).minimumShouldMatch(minShouldMatch))).get();
.type(MatchQuery.Type.PHRASE).minimumShouldMatch(minShouldMatch))).get();
} else {
left = client().prepareSearch("test").setSize(numDocs)
.addSort(SortBuilders.scoreSort()).addSort(SortBuilders.fieldSort("_id"))
Expand Down
Expand Up @@ -681,7 +681,6 @@ public void testMultiMatchQuery() throws Exception {
// this uses dismax so scores are equal and the order can be arbitrary
assertSearchHits(searchResponse, "1", "2");

builder.useDisMax(false);
searchResponse = client().prepareSearch()
.setQuery(builder)
.get();
Expand Down Expand Up @@ -786,7 +785,6 @@ public void testMultiMatchQueryMinShouldMatch() {

MultiMatchQueryBuilder multiMatchQuery = multiMatchQuery("value1 value2 foo", "field1", "field2");

multiMatchQuery.useDisMax(true);
multiMatchQuery.minimumShouldMatch("70%");
SearchResponse searchResponse = client().prepareSearch()
.setQuery(multiMatchQuery)
Expand All @@ -800,7 +798,6 @@ public void testMultiMatchQueryMinShouldMatch() {
assertFirstHit(searchResponse, hasId("1"));
assertSecondHit(searchResponse, hasId("2"));

multiMatchQuery.useDisMax(false);
multiMatchQuery.minimumShouldMatch("70%");
searchResponse = client().prepareSearch().setQuery(multiMatchQuery).get();
assertHitCount(searchResponse, 1L);
Expand Down Expand Up @@ -1475,11 +1472,11 @@ public void testMultiMatchLenientIssue3797() {
refresh();

SearchResponse searchResponse = client().prepareSearch("test")
.setQuery(multiMatchQuery("value2", "field2").field("field1", 2).lenient(true).useDisMax(false)).get();
.setQuery(multiMatchQuery("value2", "field2").field("field1", 2).lenient(true)).get();
assertHitCount(searchResponse, 1L);

searchResponse = client().prepareSearch("test")
.setQuery(multiMatchQuery("value2", "field2").field("field1", 2).lenient(true).useDisMax(true)).get();
.setQuery(multiMatchQuery("value2", "field2").field("field1", 2).lenient(true)).get();
assertHitCount(searchResponse, 1L);

searchResponse = client().prepareSearch("test")
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugin/sql/qa/src/main/resources/fulltext.csv-spec
Expand Up @@ -59,14 +59,14 @@ SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH('first_na
;

multiMatchQueryAllOptions
SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH('first_name,last_name', 'Morton', 'slop=1;lenient=true;cutoff_frequency=2;tie_breaker=0.1;use_dis_max=true;fuzzy_rewrite=scoring_boolean;minimum_should_match=1;operator=AND;max_expansions=30;prefix_length=1;analyzer=english;type=best_fields;auto_generate_synonyms_phrase_query=true;fuzzy_transpositions=true');
SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH('first_name,last_name', 'Morton', 'slop=1;lenient=true;cutoff_frequency=2;tie_breaker=0.1;fuzzy_rewrite=scoring_boolean;minimum_should_match=1;operator=AND;max_expansions=30;prefix_length=1;analyzer=english;type=best_fields;auto_generate_synonyms_phrase_query=true;fuzzy_transpositions=true');

emp_no:i | first_name:s | gender:s | last_name:s
10095 |Hilari |M |Morton
;

multiMatchQueryWithInMultipleCommaSeparatedStrings
SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH('first_name,last_name', 'Morton', 'slop=1;lenient=true', 'cutoff_frequency=2','tie_breaker=0.1;use_dis_max=true;fuzzy_rewrite=scoring_boolean','minimum_should_match=1;operator=AND;max_expansions=30;prefix_length=1;analyzer=english;type=best_fields;auto_generate_synonyms_phrase_query=true;fuzzy_transpositions=true');
SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH('first_name,last_name', 'Morton', 'slop=1;lenient=true', 'cutoff_frequency=2','tie_breaker=0.1;fuzzy_rewrite=scoring_boolean','minimum_should_match=1;operator=AND;max_expansions=30;prefix_length=1;analyzer=english;type=best_fields;auto_generate_synonyms_phrase_query=true;fuzzy_transpositions=true');

emp_no:i | first_name:s | gender:s | last_name:s
10095 |Hilari |M |Morton
Expand Down
Expand Up @@ -6,10 +6,10 @@
package org.elasticsearch.xpack.sql.querydsl.query;

import org.elasticsearch.common.Booleans;
import org.elasticsearch.index.query.Operator;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.query.MultiMatchQueryBuilder;
import org.elasticsearch.index.query.Operator;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MultiMatchQueryPredicate;
import org.elasticsearch.xpack.sql.tree.Location;

Expand All @@ -32,7 +32,6 @@ public class MultiMatchQuery extends LeafQuery {
appliers.put("lenient", (qb, s) -> qb.lenient(Booleans.parseBoolean(s)));
appliers.put("cutoff_frequency", (qb, s) -> qb.cutoffFrequency(Float.valueOf(s)));
appliers.put("tie_breaker", (qb, s) -> qb.tieBreaker(Float.valueOf(s)));
appliers.put("use_dis_max", (qb, s) -> qb.useDisMax(Booleans.parseBoolean(s)));
appliers.put("fuzzy_rewrite", (qb, s) -> qb.fuzzyRewrite(s));
appliers.put("minimum_should_match", (qb, s) -> qb.minimumShouldMatch(s));
appliers.put("operator", (qb, s) -> qb.operator(Operator.fromString(s)));
Expand Down
Expand Up @@ -23,8 +23,7 @@ public void testQueryBuilding() {
MultiMatchQueryBuilder qb = getBuilder("lenient=true");
assertThat(qb.lenient(), equalTo(true));

qb = getBuilder("use_dis_max=true;type=best_fields");
assertThat(qb.useDisMax(), equalTo(true));
qb = getBuilder("type=best_fields");
assertThat(qb.getType(), equalTo(MultiMatchQueryBuilder.Type.BEST_FIELDS));

Exception e = expectThrows(IllegalArgumentException.class, () -> getBuilder("pizza=yummy"));
Expand Down

0 comments on commit b33ff16

Please sign in to comment.