Remove deprecated `useDisMax` from MultiMatchQuery (#36488)

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.
This commit is contained in:
Christoph Büscher 2018-12-13 20:13:16 +01:00 committed by GitHub
parent 4b17055035
commit b33ff16d62
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 25 additions and 63 deletions

View File

@ -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");
@ -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;
@ -224,7 +222,9 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder<MultiMatchQuery
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();
@ -256,7 +256,9 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder<MultiMatchQuery
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);
@ -438,20 +440,6 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder<MultiMatchQuery
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
@ -593,9 +581,6 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder<MultiMatchQuery
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);
}
@ -674,8 +659,6 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder<MultiMatchQuery
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())) {
@ -724,7 +707,6 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder<MultiMatchQuery
.cutoffFrequency(cutoffFrequency)
.fuzziness(fuzziness)
.fuzzyRewrite(fuzzyRewrite)
.useDisMax(useDisMax)
.maxExpansions(maxExpansions)
.minimumShouldMatch(minimumShouldMatch)
.operator(operator)
@ -798,16 +780,6 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder<MultiMatchQuery
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
@ -828,7 +800,7 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder<MultiMatchQuery
@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);
}
@ -845,7 +817,6 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder<MultiMatchQuery
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) &&

View File

@ -118,9 +118,6 @@ public class MultiMatchQueryBuilderTests extends AbstractQueryTestCase<MultiMatc
if (randomBoolean()) {
query.fuzzyRewrite(getRandomRewriteMethod());
}
if (randomBoolean()) {
query.useDisMax(randomBoolean());
}
if (randomBoolean()) {
query.tieBreaker(randomFloat());
}
@ -189,7 +186,7 @@ public class MultiMatchQueryBuilderTests extends AbstractQueryTestCase<MultiMatc
}
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));
@ -198,8 +195,8 @@ public class MultiMatchQueryBuilderTests extends AbstractQueryTestCase<MultiMatc
}
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));
@ -209,8 +206,7 @@ public class MultiMatchQueryBuilderTests extends AbstractQueryTestCase<MultiMatc
}
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));
@ -222,7 +218,7 @@ public class MultiMatchQueryBuilderTests extends AbstractQueryTestCase<MultiMatc
}
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));

View File

@ -203,7 +203,7 @@ public class MultiMatchQueryIT extends ESIntegTestCase {
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()));
@ -323,14 +323,14 @@ public class MultiMatchQueryIT extends ESIntegTestCase {
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));
@ -399,7 +399,7 @@ public class MultiMatchQueryIT extends ESIntegTestCase {
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"))
@ -418,7 +418,7 @@ public class MultiMatchQueryIT extends ESIntegTestCase {
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"))
@ -437,7 +437,7 @@ public class MultiMatchQueryIT extends ESIntegTestCase {
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"))

View File

@ -681,7 +681,6 @@ public class SearchQueryIT extends ESIntegTestCase {
// 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();
@ -786,7 +785,6 @@ public class SearchQueryIT extends ESIntegTestCase {
MultiMatchQueryBuilder multiMatchQuery = multiMatchQuery("value1 value2 foo", "field1", "field2");
multiMatchQuery.useDisMax(true);
multiMatchQuery.minimumShouldMatch("70%");
SearchResponse searchResponse = client().prepareSearch()
.setQuery(multiMatchQuery)
@ -800,7 +798,6 @@ public class SearchQueryIT extends ESIntegTestCase {
assertFirstHit(searchResponse, hasId("1"));
assertSecondHit(searchResponse, hasId("2"));
multiMatchQuery.useDisMax(false);
multiMatchQuery.minimumShouldMatch("70%");
searchResponse = client().prepareSearch().setQuery(multiMatchQuery).get();
assertHitCount(searchResponse, 1L);
@ -1475,11 +1472,11 @@ public class SearchQueryIT extends ESIntegTestCase {
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")

View File

@ -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

View File

@ -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;
@ -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)));

View File

@ -23,8 +23,7 @@ public class MultiMatchQueryTests extends ESTestCase {
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"));