From 875927f2645d4c3fedc4aa6d01bd426677410a51 Mon Sep 17 00:00:00 2001 From: noCharger Date: Thu, 8 Dec 2022 08:52:07 -0800 Subject: [PATCH 1/3] Refactor Object to Fuzziness type for all query builders Signed-off-by: noCharger --- CHANGELOG.md | 1 + .../search/query/MultiMatchQueryIT.java | 3 +- .../search/query/SearchQueryIT.java | 11 +++---- .../org/opensearch/common/unit/Fuzziness.java | 10 +++++++ .../query/MatchBoolPrefixQueryBuilder.java | 30 +++++++++---------- .../index/query/MatchQueryBuilder.java | 10 +++---- .../index/query/MultiMatchQueryBuilder.java | 4 +-- .../index/query/QueryStringQueryBuilder.java | 5 +++- .../common/unit/FuzzinessTests.java | 12 ++++++++ .../MatchBoolPrefixQueryBuilderTests.java | 5 ++++ .../index/query/MatchQueryBuilderTests.java | 5 ++++ .../query/MultiMatchQueryBuilderTests.java | 5 ++++ .../query/QueryStringQueryBuilderTests.java | 10 +++++-- 13 files changed, 79 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e73ffdf843a4..ee8f56eb1e429 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827)) - Fixed compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944)) - Reject bulk requests with invalid actions ([#5299](https://github.com/opensearch-project/OpenSearch/issues/5299)) +- Refactor fuzziness interface on query builders ([#5433](https://github.com/opensearch-project/OpenSearch/pull/5433)) ### Security diff --git a/server/src/internalClusterTest/java/org/opensearch/search/query/MultiMatchQueryIT.java b/server/src/internalClusterTest/java/org/opensearch/search/query/MultiMatchQueryIT.java index d87bbfb1fb69c..79527039a50f5 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/query/MultiMatchQueryIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/query/MultiMatchQueryIT.java @@ -37,6 +37,7 @@ import org.opensearch.action.index.IndexRequestBuilder; import org.opensearch.action.search.SearchResponse; import org.opensearch.common.settings.Settings; +import org.opensearch.common.unit.Fuzziness; import org.opensearch.common.util.set.Sets; import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentFactory; @@ -1024,7 +1025,7 @@ public void testFuzzyFieldLevelBoosting() throws InterruptedException, Execution SearchResponse searchResponse = client().prepareSearch(idx) .setExplain(true) - .setQuery(multiMatchQuery("foo").field("title", 100).field("body").fuzziness(0)) + .setQuery(multiMatchQuery("foo").field("title", 100).field("body").fuzziness(Fuzziness.ZERO)) .get(); SearchHit[] hits = searchResponse.getHits().getHits(); assertNotEquals("both documents should be on different shards", hits[0].getShard().getShardId(), hits[1].getShard().getShardId()); diff --git a/server/src/internalClusterTest/java/org/opensearch/search/query/SearchQueryIT.java b/server/src/internalClusterTest/java/org/opensearch/search/query/SearchQueryIT.java index e90d4e8e12c10..d32487df10b38 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/query/SearchQueryIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/query/SearchQueryIT.java @@ -49,6 +49,7 @@ import org.opensearch.common.regex.Regex; import org.opensearch.common.settings.Settings; import org.opensearch.common.time.DateFormatter; +import org.opensearch.common.unit.Fuzziness; import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.XContentType; @@ -762,21 +763,21 @@ public void testMatchQueryFuzzy() throws Exception { client().prepareIndex("test").setId("2").setSource("text", "Unity") ); - SearchResponse searchResponse = client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness("0")).get(); + SearchResponse searchResponse = client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness(Fuzziness.ZERO)).get(); assertHitCount(searchResponse, 0L); - searchResponse = client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness("1")).get(); + searchResponse = client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness(Fuzziness.ONE)).get(); assertHitCount(searchResponse, 2L); assertSearchHits(searchResponse, "1", "2"); - searchResponse = client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness("AUTO")).get(); + searchResponse = client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness(Fuzziness.AUTO)).get(); assertHitCount(searchResponse, 2L); assertSearchHits(searchResponse, "1", "2"); - searchResponse = client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness("AUTO:5,7")).get(); + searchResponse = client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness(Fuzziness.customAuto(5, 7))).get(); assertHitCount(searchResponse, 0L); - searchResponse = client().prepareSearch().setQuery(matchQuery("text", "unify").fuzziness("AUTO:5,7")).get(); + searchResponse = client().prepareSearch().setQuery(matchQuery("text", "unify").fuzziness(Fuzziness.customAuto(5, 7))).get(); assertHitCount(searchResponse, 1L); assertSearchHits(searchResponse, "2"); } diff --git a/server/src/main/java/org/opensearch/common/unit/Fuzziness.java b/server/src/main/java/org/opensearch/common/unit/Fuzziness.java index c3b6ea6b8c23d..28947b3936843 100644 --- a/server/src/main/java/org/opensearch/common/unit/Fuzziness.java +++ b/server/src/main/java/org/opensearch/common/unit/Fuzziness.java @@ -139,6 +139,16 @@ public static Fuzziness build(Object fuzziness) { return new Fuzziness(string); } + /*** + * Creates a {@link Fuzziness} instance from lowDistance and highDistance. + * where the edit distance is 0 for strings shorter than lowDistance, + * 1 for strings where its length between lowDistance and highDistance (inclusive), + * and 2 for strings longer than highDistance. + */ + public static Fuzziness customAuto(int lowDistance, int highDistance) { + return new Fuzziness("AUTO", lowDistance, highDistance); + } + private static Fuzziness parseCustomAuto(final String string) { assert string.toUpperCase(Locale.ROOT).startsWith(AUTO.asString() + ":"); String[] fuzzinessLimit = string.substring(AUTO.asString().length() + 1).split(","); diff --git a/server/src/main/java/org/opensearch/index/query/MatchBoolPrefixQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/MatchBoolPrefixQueryBuilder.java index f8f84c52309d5..531bdae3edb9b 100644 --- a/server/src/main/java/org/opensearch/index/query/MatchBoolPrefixQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MatchBoolPrefixQueryBuilder.java @@ -176,8 +176,10 @@ public String minimumShouldMatch() { } /** Sets the fuzziness used when evaluated to a fuzzy query type. Defaults to "AUTO". */ - public MatchBoolPrefixQueryBuilder fuzziness(Object fuzziness) { - this.fuzziness = Fuzziness.build(fuzziness); + public MatchBoolPrefixQueryBuilder fuzziness(Fuzziness fuzziness) { + if (fuzziness != null) { + this.fuzziness = fuzziness; + } return this; } @@ -348,19 +350,17 @@ public static MatchBoolPrefixQueryBuilder fromXContent(XContentParser parser) th } } - MatchBoolPrefixQueryBuilder queryBuilder = new MatchBoolPrefixQueryBuilder(fieldName, value); - queryBuilder.analyzer(analyzer); - queryBuilder.operator(operator); - queryBuilder.minimumShouldMatch(minimumShouldMatch); - queryBuilder.boost(boost); - queryBuilder.queryName(queryName); - if (fuzziness != null) { - queryBuilder.fuzziness(fuzziness); - } - queryBuilder.prefixLength(prefixLength); - queryBuilder.maxExpansions(maxExpansion); - queryBuilder.fuzzyTranspositions(fuzzyTranspositions); - queryBuilder.fuzzyRewrite(fuzzyRewrite); + MatchBoolPrefixQueryBuilder queryBuilder = new MatchBoolPrefixQueryBuilder(fieldName, value) + .analyzer(analyzer) + .operator(operator) + .minimumShouldMatch(minimumShouldMatch) + .boost(boost) + .queryName(queryName) + .fuzziness(fuzziness) + .prefixLength(prefixLength) + .maxExpansions(maxExpansion) + .fuzzyTranspositions(fuzzyTranspositions) + .fuzzyRewrite(fuzzyRewrite); return queryBuilder; } diff --git a/server/src/main/java/org/opensearch/index/query/MatchQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/MatchQueryBuilder.java index 380e8722daca9..32b7027cb9e43 100644 --- a/server/src/main/java/org/opensearch/index/query/MatchQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MatchQueryBuilder.java @@ -209,8 +209,10 @@ public String analyzer() { } /** Sets the fuzziness used when evaluated to a fuzzy query type. Defaults to "AUTO". */ - public MatchQueryBuilder fuzziness(Object fuzziness) { - this.fuzziness = Fuzziness.build(fuzziness); + public MatchQueryBuilder fuzziness(Fuzziness fuzziness) { + if (fuzziness != null) { + this.fuzziness = fuzziness; + } return this; } @@ -565,9 +567,7 @@ public static MatchQueryBuilder fromXContent(XContentParser parser) throws IOExc matchQuery.operator(operator); matchQuery.analyzer(analyzer); matchQuery.minimumShouldMatch(minimumShouldMatch); - if (fuzziness != null) { - matchQuery.fuzziness(fuzziness); - } + matchQuery.fuzziness(fuzziness); matchQuery.fuzzyRewrite(fuzzyRewrite); matchQuery.prefixLength(prefixLength); matchQuery.fuzzyTranspositions(fuzzyTranspositions); diff --git a/server/src/main/java/org/opensearch/index/query/MultiMatchQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/MultiMatchQueryBuilder.java index fe3bcd81e72be..8f16346a15991 100644 --- a/server/src/main/java/org/opensearch/index/query/MultiMatchQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MultiMatchQueryBuilder.java @@ -400,9 +400,9 @@ public int slop() { /** * Sets the fuzziness used when evaluated to a fuzzy query type. Defaults to "AUTO". */ - public MultiMatchQueryBuilder fuzziness(Object fuzziness) { + public MultiMatchQueryBuilder fuzziness(Fuzziness fuzziness) { if (fuzziness != null) { - this.fuzziness = Fuzziness.build(fuzziness); + this.fuzziness = fuzziness; } return this; } diff --git a/server/src/main/java/org/opensearch/index/query/QueryStringQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/QueryStringQueryBuilder.java index 32337f5df34c5..519d5a6916336 100644 --- a/server/src/main/java/org/opensearch/index/query/QueryStringQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/QueryStringQueryBuilder.java @@ -79,6 +79,7 @@ public class QueryStringQueryBuilder extends AbstractQueryBuilder Date: Thu, 8 Dec 2022 09:58:17 -0800 Subject: [PATCH 2/3] Revise on bwc Signed-off-by: noCharger --- .../index/query/MatchBoolPrefixQueryBuilder.java | 14 +++++++++----- .../opensearch/index/query/MatchQueryBuilder.java | 11 ++++++++--- .../index/query/MultiMatchQueryBuilder.java | 13 +++++++++++-- .../index/query/QueryStringQueryBuilder.java | 4 +--- .../index/query/QueryStringQueryBuilderTests.java | 2 +- 5 files changed, 30 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/query/MatchBoolPrefixQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/MatchBoolPrefixQueryBuilder.java index 531bdae3edb9b..f901fac22d7ae 100644 --- a/server/src/main/java/org/opensearch/index/query/MatchBoolPrefixQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MatchBoolPrefixQueryBuilder.java @@ -175,11 +175,16 @@ public String minimumShouldMatch() { return this.minimumShouldMatch; } + @Deprecated + /** Sets the fuzziness used when evaluated to a fuzzy query type. Defaults to "AUTO". */ + public MatchBoolPrefixQueryBuilder fuzziness(Object fuzziness) { + this.fuzziness = Fuzziness.build(fuzziness); + return this; + } + /** Sets the fuzziness used when evaluated to a fuzzy query type. Defaults to "AUTO". */ public MatchBoolPrefixQueryBuilder fuzziness(Fuzziness fuzziness) { - if (fuzziness != null) { - this.fuzziness = fuzziness; - } + this.fuzziness = fuzziness; return this; } @@ -350,8 +355,7 @@ public static MatchBoolPrefixQueryBuilder fromXContent(XContentParser parser) th } } - MatchBoolPrefixQueryBuilder queryBuilder = new MatchBoolPrefixQueryBuilder(fieldName, value) - .analyzer(analyzer) + MatchBoolPrefixQueryBuilder queryBuilder = new MatchBoolPrefixQueryBuilder(fieldName, value).analyzer(analyzer) .operator(operator) .minimumShouldMatch(minimumShouldMatch) .boost(boost) diff --git a/server/src/main/java/org/opensearch/index/query/MatchQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/MatchQueryBuilder.java index 32b7027cb9e43..8dbe9392bdd95 100644 --- a/server/src/main/java/org/opensearch/index/query/MatchQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MatchQueryBuilder.java @@ -208,11 +208,16 @@ public String analyzer() { return this.analyzer; } + @Deprecated + /** Sets the fuzziness used when evaluated to a fuzzy query type. Defaults to "AUTO". */ + public MatchQueryBuilder fuzziness(Object fuzziness) { + this.fuzziness = Fuzziness.build(fuzziness); + return this; + } + /** Sets the fuzziness used when evaluated to a fuzzy query type. Defaults to "AUTO". */ public MatchQueryBuilder fuzziness(Fuzziness fuzziness) { - if (fuzziness != null) { - this.fuzziness = fuzziness; - } + this.fuzziness = fuzziness; return this; } diff --git a/server/src/main/java/org/opensearch/index/query/MultiMatchQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/MultiMatchQueryBuilder.java index 8f16346a15991..2270c3675fa11 100644 --- a/server/src/main/java/org/opensearch/index/query/MultiMatchQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MultiMatchQueryBuilder.java @@ -397,16 +397,25 @@ public int slop() { return slop; } + @Deprecated /** * Sets the fuzziness used when evaluated to a fuzzy query type. Defaults to "AUTO". */ - public MultiMatchQueryBuilder fuzziness(Fuzziness fuzziness) { + public MultiMatchQueryBuilder fuzziness(Object fuzziness) { if (fuzziness != null) { - this.fuzziness = fuzziness; + this.fuzziness = Fuzziness.build(fuzziness); } return this; } + /** + * Sets the fuzziness used when evaluated to a fuzzy query type. Defaults to "AUTO". + */ + public MultiMatchQueryBuilder fuzziness(Fuzziness fuzziness) { + this.fuzziness = fuzziness; + return this; + } + public Fuzziness fuzziness() { return fuzziness; } diff --git a/server/src/main/java/org/opensearch/index/query/QueryStringQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/QueryStringQueryBuilder.java index 519d5a6916336..4ee790291f453 100644 --- a/server/src/main/java/org/opensearch/index/query/QueryStringQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/QueryStringQueryBuilder.java @@ -417,9 +417,7 @@ public boolean enablePositionIncrements() { * Set the edit distance for fuzzy queries. Default is "AUTO". */ public QueryStringQueryBuilder fuzziness(Fuzziness fuzziness) { - if (fuzziness != null) { - this.fuzziness = fuzziness; - } + this.fuzziness = fuzziness; return this; } diff --git a/server/src/test/java/org/opensearch/index/query/QueryStringQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/QueryStringQueryBuilderTests.java index d5fc72ae6f4c6..ad39cd831a30c 100644 --- a/server/src/test/java/org/opensearch/index/query/QueryStringQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/query/QueryStringQueryBuilderTests.java @@ -869,7 +869,7 @@ public void testFuzzyNumeric() throws Exception { public void testDefaultFuzziness() { QueryStringQueryBuilder queryStringQueryBuilder = new QueryStringQueryBuilder(TEXT_FIELD_NAME).fuzziness(null); - assertEquals(QueryStringQueryBuilder.DEFAULT_FUZZINESS, queryStringQueryBuilder.fuzziness()); + assertNull(queryStringQueryBuilder.fuzziness()); } public void testPrefixNumeric() throws Exception { From 2ddb53226a9a8626b3faae9a53e18325abfa81ab Mon Sep 17 00:00:00 2001 From: noCharger Date: Mon, 12 Dec 2022 16:21:24 -0800 Subject: [PATCH 3/3] Update change log Signed-off-by: noCharger --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab5bf834ada87..b7175e97a8560 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,7 +82,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fixed compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944)) - Reject bulk requests with invalid actions ([#5299](https://github.com/opensearch-project/OpenSearch/issues/5299)) - Support OpenSSL Provider with default Netty allocator ([#5460](https://github.com/opensearch-project/OpenSearch/pull/5460)) -- Refactor fuzziness interface on query builders ([#5433](https://github.com/opensearch-project/OpenSearch/pull/5433)) ### Security @@ -100,6 +99,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Bump gradle-extra-configurations-plugin from 7.0.0 to 8.0.0 ([#4808](https://github.com/opensearch-project/OpenSearch/pull/4808)) ### Changed ### Deprecated +- Refactor fuzziness interface on query builders ([#5433](https://github.com/opensearch-project/OpenSearch/pull/5433)) + ### Removed ### Fixed - Fix 1.x compatibility bug with stored Tasks ([#5412](https://github.com/opensearch-project/OpenSearch/pull/5412))