From b33ff16d62e57843c407271d3aadeb6328567441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 13 Dec 2018 20:13:16 +0100 Subject: [PATCH] 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. --- .../index/query/MultiMatchQueryBuilder.java | 43 +++---------------- .../query/MultiMatchQueryBuilderTests.java | 14 +++--- .../search/query/MultiMatchQueryIT.java | 12 +++--- .../search/query/SearchQueryIT.java | 7 +-- .../qa/src/main/resources/fulltext.csv-spec | 4 +- .../sql/querydsl/query/MultiMatchQuery.java | 5 +-- .../querydsl/query/MultiMatchQueryTests.java | 3 +- 7 files changed, 25 insertions(+), 63 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java index 996e878dba85c..d8476d791d7ec 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java @@ -67,7 +67,6 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilderTie-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 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); } @@ -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())) { @@ -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) @@ -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 newFieldsBoosts; if (fieldsBoosts.isEmpty()) { // no fields provided, defaults to index.query.default_field @@ -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); } @@ -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) && diff --git a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java index 58604f7e83c47..43c76f028e22e 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java @@ -118,9 +118,6 @@ protected MultiMatchQueryBuilder doCreateTestQueryBuilder() { if (randomBoolean()) { query.fuzzyRewrite(getRandomRewriteMethod()); } - if (randomBoolean()) { - query.useDisMax(randomBoolean()); - } if (randomBoolean()) { query.tieBreaker(randomFloat()); } @@ -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)); @@ -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)); @@ -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)); @@ -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)); diff --git a/server/src/test/java/org/elasticsearch/search/query/MultiMatchQueryIT.java b/server/src/test/java/org/elasticsearch/search/query/MultiMatchQueryIT.java index f5688e0a4215e..76850a197d969 100644 --- a/server/src/test/java/org/elasticsearch/search/query/MultiMatchQueryIT.java +++ b/server/src/test/java/org/elasticsearch/search/query/MultiMatchQueryIT.java @@ -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())); @@ -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)); @@ -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")) @@ -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")) @@ -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")) diff --git a/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java b/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java index ebfd4dbbebf29..2b6b8b1c60bcc 100644 --- a/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java +++ b/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java @@ -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(); @@ -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) @@ -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); @@ -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") diff --git a/x-pack/plugin/sql/qa/src/main/resources/fulltext.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/fulltext.csv-spec index 93493ffdc2acb..0cfcf743414b9 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/fulltext.csv-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/fulltext.csv-spec @@ -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 diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQuery.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQuery.java index 81c990f85bdb4..30def4db3dac4 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQuery.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQuery.java @@ -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))); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQueryTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQueryTests.java index ba2d548cde9dd..2e26c7c0595c2 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQueryTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQueryTests.java @@ -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"));