From 9a55cd7104b2e21327d652c1fe5921b963f0defb Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Tue, 29 Aug 2023 12:19:42 -0700 Subject: [PATCH 01/13] Add support for in KNNQueryBuilder Signed-off-by: Ryan Bogan --- .../knn/index/query/KNNQueryBuilder.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java index 88c5c30f1..86449e5d7 100644 --- a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java +++ b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java @@ -6,6 +6,7 @@ package org.opensearch.knn.index.query; import lombok.extern.log4j.Log4j2; +import org.apache.lucene.search.MatchNoDocsQuery; import org.opensearch.core.common.Strings; import org.opensearch.index.mapper.NumberFieldMapper; import org.opensearch.index.query.QueryBuilder; @@ -43,6 +44,7 @@ public class KNNQueryBuilder extends AbstractQueryBuilder { public static final ParseField VECTOR_FIELD = new ParseField("vector"); public static final ParseField K_FIELD = new ParseField("k"); public static final ParseField FILTER_FIELD = new ParseField("filter"); + public static final ParseField IGNORE_UNMAPPED_FIELD = new ParseField("ignore_unmapped"); public static int K_MAX = 10000; /** * The name for the knn query @@ -55,6 +57,7 @@ public class KNNQueryBuilder extends AbstractQueryBuilder { private final float[] vector; private int k = 0; private QueryBuilder filter; + private boolean ignoreUnmapped = false; /** * Constructs a new knn query @@ -88,6 +91,7 @@ public KNNQueryBuilder(String fieldName, float[] vector, int k, QueryBuilder fil this.vector = vector; this.k = k; this.filter = filter; + this.ignoreUnmapped = false; } public static void initialize(ModelDao modelDao) { @@ -113,6 +117,7 @@ public KNNQueryBuilder(StreamInput in) throws IOException { vector = in.readFloatArray(); k = in.readInt(); filter = in.readOptionalNamedWriteable(QueryBuilder.class); + ignoreUnmapped = in.readBoolean(); } catch (IOException ex) { throw new RuntimeException("[KNN] Unable to create KNNQueryBuilder", ex); } @@ -126,6 +131,7 @@ public static KNNQueryBuilder fromXContent(XContentParser parser) throws IOExcep QueryBuilder filter = null; String queryName = null; String currentFieldName = null; + boolean ignoreUnmapped = false; XContentParser.Token token; KNNCounter.KNN_QUERY_REQUESTS.increment(); while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { @@ -144,6 +150,8 @@ public static KNNQueryBuilder fromXContent(XContentParser parser) throws IOExcep boost = parser.floatValue(); } else if (K_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { k = (Integer) NumberFieldMapper.NumberType.INTEGER.parse(parser.objectBytes(), false); + } else if (IGNORE_UNMAPPED_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { + ignoreUnmapped = parser.booleanValue(); } else if (AbstractQueryBuilder.NAME_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { queryName = parser.text(); } else { @@ -176,6 +184,7 @@ public static KNNQueryBuilder fromXContent(XContentParser parser) throws IOExcep } KNNQueryBuilder knnQueryBuilder = new KNNQueryBuilder(fieldName, ObjectsToFloats(vector), k, filter); + knnQueryBuilder.ignoreUnmapped(ignoreUnmapped); knnQueryBuilder.queryName(queryName); knnQueryBuilder.boost(boost); return knnQueryBuilder; @@ -187,6 +196,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { out.writeFloatArray(vector); out.writeInt(k); out.writeOptionalNamedWriteable(filter); + out.writeBoolean(ignoreUnmapped); } /** @@ -211,6 +221,18 @@ public QueryBuilder getFilter() { return this.filter; } + /** + * Sets whether the query builder should ignore unmapped paths (and run a + * {@link MatchNoDocsQuery} in place of this query) or throw an exception if + * the path is unmapped. + */ + public KNNQueryBuilder ignoreUnmapped(boolean ignoreUnmapped) { + this.ignoreUnmapped = ignoreUnmapped; + return this; + } + + public boolean isIgnoreUnmapped() { return this.ignoreUnmapped; } + @Override public void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); @@ -221,6 +243,7 @@ public void doXContent(XContentBuilder builder, Params params) throws IOExceptio if (filter != null) { builder.field(FILTER_FIELD.getPreferredName(), filter); } + builder.field(IGNORE_UNMAPPED_FIELD.getPreferredName(), ignoreUnmapped); printBoostAndQueryName(builder); builder.endObject(); builder.endObject(); @@ -230,6 +253,12 @@ public void doXContent(XContentBuilder builder, Params params) throws IOExceptio protected Query doToQuery(QueryShardContext context) { MappedFieldType mappedFieldType = context.fieldMapper(this.fieldName); + if (mappedFieldType == null) { + if (ignoreUnmapped) { + return new MatchNoDocsQuery(); + } + } + if (!(mappedFieldType instanceof KNNVectorFieldMapper.KNNVectorFieldType)) { throw new IllegalArgumentException(String.format("Field '%s' is not knn_vector type.", this.fieldName)); } From b0252b91b8327eb1a5216e70893c105768309fc4 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Tue, 29 Aug 2023 13:13:42 -0700 Subject: [PATCH 02/13] Fix spotless Signed-off-by: Ryan Bogan --- .../opensearch/knn/index/query/KNNQueryBuilder.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java index 86449e5d7..d6a879e5a 100644 --- a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java +++ b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java @@ -231,7 +231,9 @@ public KNNQueryBuilder ignoreUnmapped(boolean ignoreUnmapped) { return this; } - public boolean isIgnoreUnmapped() { return this.ignoreUnmapped; } + public boolean isIgnoreUnmapped() { + return this.ignoreUnmapped; + } @Override public void doXContent(XContentBuilder builder, Params params) throws IOException { @@ -253,10 +255,8 @@ public void doXContent(XContentBuilder builder, Params params) throws IOExceptio protected Query doToQuery(QueryShardContext context) { MappedFieldType mappedFieldType = context.fieldMapper(this.fieldName); - if (mappedFieldType == null) { - if (ignoreUnmapped) { - return new MatchNoDocsQuery(); - } + if (mappedFieldType == null && ignoreUnmapped) { + return new MatchNoDocsQuery(); } if (!(mappedFieldType instanceof KNNVectorFieldMapper.KNNVectorFieldType)) { From 7687a0b4f2ee910819afd97d3d557364b59768ba Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Wed, 30 Aug 2023 12:32:26 -0700 Subject: [PATCH 03/13] Add test Signed-off-by: Ryan Bogan --- .../opensearch/knn/index/query/KNNQueryBuilder.java | 2 +- .../knn/index/query/KNNQueryBuilderTests.java | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java index d6a879e5a..6681b951f 100644 --- a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java +++ b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java @@ -231,7 +231,7 @@ public KNNQueryBuilder ignoreUnmapped(boolean ignoreUnmapped) { return this; } - public boolean isIgnoreUnmapped() { + public boolean getIgnoreUnmapped() { return this.ignoreUnmapped; } diff --git a/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java b/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java index 4e7f739a7..f8d16e3a7 100644 --- a/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java +++ b/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java @@ -7,6 +7,7 @@ import com.google.common.collect.ImmutableMap; import org.apache.lucene.search.KnnFloatVectorQuery; +import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.opensearch.Version; import org.opensearch.cluster.ClusterModule; @@ -41,6 +42,7 @@ import java.util.List; import java.util.Optional; +import static org.hamcrest.Matchers.instanceOf; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -307,4 +309,13 @@ private void assertSerialization(final Version version, final Optional Date: Thu, 31 Aug 2023 09:12:13 -0700 Subject: [PATCH 04/13] Add Changelog entry Signed-off-by: Ryan Bogan --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6f37f6d1..bc53414ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * Enabled the IVF algorithm to work with Filters of K-NN Query. [#1013](https://github.com/opensearch-project/k-NN/pull/1013) * Improved the logic to switch to exact search for restrictive filters search for better recall. [#1059](https://github.com/opensearch-project/k-NN/pull/1059) * Added max distance computation logic to enhance the switch to exact search in filtered Nearest Neighbor Search. [#1066](https://github.com/opensearch-project/k-NN/pull/1066) +* Added support for ignore_unmapped in KNN queries. [#1071](https://github.com/opensearch-project/k-NN/pull/1071) ### Bug Fixes ### Infrastructure ### Documentation From f97681dcf2b6e7655c682aa4891e84d87d337cd3 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Fri, 1 Sep 2023 10:31:59 -0700 Subject: [PATCH 05/13] Change StreamInput reading for ignoreUnmapped to optional Signed-off-by: Ryan Bogan --- .../java/org/opensearch/knn/index/query/KNNQueryBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java index 6681b951f..fffa5d419 100644 --- a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java +++ b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java @@ -117,7 +117,7 @@ public KNNQueryBuilder(StreamInput in) throws IOException { vector = in.readFloatArray(); k = in.readInt(); filter = in.readOptionalNamedWriteable(QueryBuilder.class); - ignoreUnmapped = in.readBoolean(); + ignoreUnmapped = in.readOptionalBoolean(); } catch (IOException ex) { throw new RuntimeException("[KNN] Unable to create KNNQueryBuilder", ex); } From bd5567350cbf5b831c6890b87b90b79290afba9a Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Mon, 18 Sep 2023 15:21:35 -0700 Subject: [PATCH 06/13] Update optional reading and writing for backwards compatibility Signed-off-by: Ryan Bogan --- .../knn/index/query/KNNQueryBuilder.java | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java index fffa5d419..0e6cac1fe 100644 --- a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java +++ b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java @@ -7,9 +7,11 @@ import lombok.extern.log4j.Log4j2; import org.apache.lucene.search.MatchNoDocsQuery; +import org.opensearch.Version; import org.opensearch.core.common.Strings; import org.opensearch.index.mapper.NumberFieldMapper; import org.opensearch.index.query.QueryBuilder; +import org.opensearch.knn.index.KNNClusterUtil; import org.opensearch.knn.index.KNNMethodContext; import org.opensearch.knn.index.VectorDataType; import org.opensearch.knn.index.mapper.KNNVectorFieldMapper; @@ -29,7 +31,9 @@ import org.opensearch.index.query.QueryShardContext; import java.io.IOException; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import static org.opensearch.knn.index.mapper.KNNVectorFieldMapperUtil.validateByteVectorValue; @@ -58,6 +62,12 @@ public class KNNQueryBuilder extends AbstractQueryBuilder { private int k = 0; private QueryBuilder filter; private boolean ignoreUnmapped = false; + private static final Version MINIMAL_SUPPORTED_VERSION_FOR_IGNORE_UNMAPPED = Version.V_2_11_0; + private static final Map minimalRequiredVersionMap = new HashMap() { + { + put("ignore_unmapped", MINIMAL_SUPPORTED_VERSION_FOR_IGNORE_UNMAPPED); + } + }; /** * Constructs a new knn query @@ -117,7 +127,9 @@ public KNNQueryBuilder(StreamInput in) throws IOException { vector = in.readFloatArray(); k = in.readInt(); filter = in.readOptionalNamedWriteable(QueryBuilder.class); - ignoreUnmapped = in.readOptionalBoolean(); + if (isClusterOnOrAfterMinRequiredVersion("ignore_unmapped")) { + ignoreUnmapped = in.readOptionalBoolean(); + } } catch (IOException ex) { throw new RuntimeException("[KNN] Unable to create KNNQueryBuilder", ex); } @@ -151,7 +163,9 @@ public static KNNQueryBuilder fromXContent(XContentParser parser) throws IOExcep } else if (K_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { k = (Integer) NumberFieldMapper.NumberType.INTEGER.parse(parser.objectBytes(), false); } else if (IGNORE_UNMAPPED_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { - ignoreUnmapped = parser.booleanValue(); + if (isClusterOnOrAfterMinRequiredVersion("ignore_unmapped")) { + ignoreUnmapped = parser.booleanValue(); + } } else if (AbstractQueryBuilder.NAME_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { queryName = parser.text(); } else { @@ -196,7 +210,9 @@ protected void doWriteTo(StreamOutput out) throws IOException { out.writeFloatArray(vector); out.writeInt(k); out.writeOptionalNamedWriteable(filter); - out.writeBoolean(ignoreUnmapped); + if (isClusterOnOrAfterMinRequiredVersion("ignore_unmapped")) { + out.writeOptionalBoolean(ignoreUnmapped); + } } /** @@ -343,4 +359,12 @@ protected int doHashCode() { public String getWriteableName() { return NAME; } + + private static boolean isClusterOnOrAfterMinRequiredVersion(String key) { + Version minimalRequiredVersion = minimalRequiredVersionMap.get(key); + if (minimalRequiredVersion == null) { + return false; + } + return KNNClusterUtil.instance().getClusterMinVersion().onOrAfter(minimalRequiredVersion); + } } From 2d5a9f4fa4d83fa9452084ee552bcb86ffd6dfc8 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Mon, 18 Sep 2023 16:10:25 -0700 Subject: [PATCH 07/13] Minor change Signed-off-by: Ryan Bogan --- .../org/opensearch/knn/index/query/KNNQueryBuilder.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java index 0e6cac1fe..2862cb65f 100644 --- a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java +++ b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java @@ -162,7 +162,7 @@ public static KNNQueryBuilder fromXContent(XContentParser parser) throws IOExcep boost = parser.floatValue(); } else if (K_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { k = (Integer) NumberFieldMapper.NumberType.INTEGER.parse(parser.objectBytes(), false); - } else if (IGNORE_UNMAPPED_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { + } else if (IGNORE_UNMAPPED_FIELD.getPreferredName().equals(currentFieldName)) { if (isClusterOnOrAfterMinRequiredVersion("ignore_unmapped")) { ignoreUnmapped = parser.booleanValue(); } @@ -261,7 +261,9 @@ public void doXContent(XContentBuilder builder, Params params) throws IOExceptio if (filter != null) { builder.field(FILTER_FIELD.getPreferredName(), filter); } - builder.field(IGNORE_UNMAPPED_FIELD.getPreferredName(), ignoreUnmapped); + if (isClusterOnOrAfterMinRequiredVersion("ignore_unmapped")) { + builder.field(IGNORE_UNMAPPED_FIELD.getPreferredName(), ignoreUnmapped); + } printBoostAndQueryName(builder); builder.endObject(); builder.endObject(); From 313fca94ee919496820d8fe34b6e73255dba32fc Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Tue, 19 Sep 2023 10:14:20 -0700 Subject: [PATCH 08/13] Debugging Signed-off-by: Ryan Bogan --- .../java/org/opensearch/knn/index/query/KNNQueryBuilder.java | 2 +- .../org/opensearch/knn/index/query/KNNQueryBuilderTests.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java index 2862cb65f..f12316d45 100644 --- a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java +++ b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java @@ -162,7 +162,7 @@ public static KNNQueryBuilder fromXContent(XContentParser parser) throws IOExcep boost = parser.floatValue(); } else if (K_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { k = (Integer) NumberFieldMapper.NumberType.INTEGER.parse(parser.objectBytes(), false); - } else if (IGNORE_UNMAPPED_FIELD.getPreferredName().equals(currentFieldName)) { + } else if ((IGNORE_UNMAPPED_FIELD.getPreferredName()).equals(currentFieldName)) { if (isClusterOnOrAfterMinRequiredVersion("ignore_unmapped")) { ignoreUnmapped = parser.booleanValue(); } diff --git a/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java b/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java index f8d16e3a7..fd5f9a1ba 100644 --- a/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java +++ b/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java @@ -48,6 +48,7 @@ import static org.mockito.Mockito.when; import static org.opensearch.knn.common.KNNConstants.METHOD_HNSW; import static org.opensearch.knn.index.KNNClusterTestUtils.mockClusterService; +import static org.opensearch.knn.index.query.KNNQueryBuilder.IGNORE_UNMAPPED_FIELD; public class KNNQueryBuilderTests extends KNNTestCase { @@ -314,6 +315,7 @@ public void testIgnoreUnmapped() throws IOException { float[] queryVector = { 1.0f, 2.0f, 3.0f, 4.0f }; KNNQueryBuilder knnQueryBuilder = new KNNQueryBuilder(FIELD_NAME, queryVector, K); knnQueryBuilder.ignoreUnmapped(true); + assertTrue(knnQueryBuilder.getIgnoreUnmapped()); Query query = knnQueryBuilder.doToQuery(mock(QueryShardContext.class)); assertNotNull(query); assertThat(query, instanceOf(MatchNoDocsQuery.class)); From 101ad8b4c82d7caa52d79592c09397aff2e03a03 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Tue, 19 Sep 2023 10:33:10 -0700 Subject: [PATCH 09/13] Debugging Signed-off-by: Ryan Bogan --- .../java/org/opensearch/knn/index/query/KNNQueryBuilder.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java index f12316d45..da2e51880 100644 --- a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java +++ b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java @@ -152,6 +152,8 @@ public static KNNQueryBuilder fromXContent(XContentParser parser) throws IOExcep } else if (token == XContentParser.Token.START_OBJECT) { throwParsingExceptionOnMultipleFields(NAME, parser.getTokenLocation(), fieldName, currentFieldName); fieldName = currentFieldName; + System.out.println(currentFieldName); + System.out.println(IGNORE_UNMAPPED_FIELD.getPreferredName()); while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); @@ -162,7 +164,7 @@ public static KNNQueryBuilder fromXContent(XContentParser parser) throws IOExcep boost = parser.floatValue(); } else if (K_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { k = (Integer) NumberFieldMapper.NumberType.INTEGER.parse(parser.objectBytes(), false); - } else if ((IGNORE_UNMAPPED_FIELD.getPreferredName()).equals(currentFieldName)) { + } else if (IGNORE_UNMAPPED_FIELD.getPreferredName().equals(currentFieldName)) { if (isClusterOnOrAfterMinRequiredVersion("ignore_unmapped")) { ignoreUnmapped = parser.booleanValue(); } From 8f64524114d464fc5c7690fc1d169194cd402b19 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Tue, 26 Sep 2023 16:21:00 -0700 Subject: [PATCH 10/13] Minor change Signed-off-by: Ryan Bogan --- .../java/org/opensearch/knn/index/query/KNNQueryBuilder.java | 2 +- .../org/opensearch/knn/index/query/KNNQueryBuilderTests.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java index da2e51880..4d039422e 100644 --- a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java +++ b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java @@ -263,7 +263,7 @@ public void doXContent(XContentBuilder builder, Params params) throws IOExceptio if (filter != null) { builder.field(FILTER_FIELD.getPreferredName(), filter); } - if (isClusterOnOrAfterMinRequiredVersion("ignore_unmapped")) { + if (ignoreUnmapped) { builder.field(IGNORE_UNMAPPED_FIELD.getPreferredName(), ignoreUnmapped); } printBoostAndQueryName(builder); diff --git a/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java b/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java index fd5f9a1ba..0d0786e11 100644 --- a/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java +++ b/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java @@ -48,7 +48,6 @@ import static org.mockito.Mockito.when; import static org.opensearch.knn.common.KNNConstants.METHOD_HNSW; import static org.opensearch.knn.index.KNNClusterTestUtils.mockClusterService; -import static org.opensearch.knn.index.query.KNNQueryBuilder.IGNORE_UNMAPPED_FIELD; public class KNNQueryBuilderTests extends KNNTestCase { From 76b185056e18448a35ba25f8a336de16f2bee40a Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Tue, 26 Sep 2023 16:35:41 -0700 Subject: [PATCH 11/13] Minor change Signed-off-by: Ryan Bogan --- .../org/opensearch/knn/index/query/KNNQueryBuilderTests.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java b/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java index 0d0786e11..959740e5a 100644 --- a/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java +++ b/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java @@ -318,5 +318,8 @@ public void testIgnoreUnmapped() throws IOException { Query query = knnQueryBuilder.doToQuery(mock(QueryShardContext.class)); assertNotNull(query); assertThat(query, instanceOf(MatchNoDocsQuery.class)); + knnQueryBuilder.ignoreUnmapped(false); + expectThrows(IllegalArgumentException.class, () -> knnQueryBuilder.doToQuery(mock(QueryShardContext.class))); + } } From 278e7b2b0df8284f50ccfc5c707a8b9681d5ecae Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Tue, 26 Sep 2023 16:50:52 -0700 Subject: [PATCH 12/13] Add test Signed-off-by: Ryan Bogan --- .../org/opensearch/knn/index/query/KNNQueryBuilderTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java b/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java index 959740e5a..e540725fc 100644 --- a/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java +++ b/src/test/java/org/opensearch/knn/index/query/KNNQueryBuilderTests.java @@ -320,6 +320,5 @@ public void testIgnoreUnmapped() throws IOException { assertThat(query, instanceOf(MatchNoDocsQuery.class)); knnQueryBuilder.ignoreUnmapped(false); expectThrows(IllegalArgumentException.class, () -> knnQueryBuilder.doToQuery(mock(QueryShardContext.class))); - } } From c3211e8367a0d434b949ea2b1c292ee7bbd81335 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Wed, 27 Sep 2023 11:30:18 -0700 Subject: [PATCH 13/13] Move cluster version check to IndexUtil Signed-off-by: Ryan Bogan --- .../org/opensearch/knn/index/IndexUtil.java | 17 +++++++++++++++++ .../knn/index/query/KNNQueryBuilder.java | 19 +------------------ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/opensearch/knn/index/IndexUtil.java b/src/main/java/org/opensearch/knn/index/IndexUtil.java index 60156b4a7..574e4a977 100644 --- a/src/main/java/org/opensearch/knn/index/IndexUtil.java +++ b/src/main/java/org/opensearch/knn/index/IndexUtil.java @@ -13,6 +13,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; +import org.opensearch.Version; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.MappingMetadata; import org.opensearch.common.ValidationException; @@ -24,6 +25,7 @@ import java.io.File; import java.util.Collections; +import java.util.HashMap; import java.util.Map; import static org.opensearch.knn.common.KNNConstants.BYTES_PER_KILOBYTES; @@ -32,6 +34,13 @@ public class IndexUtil { + private static final Version MINIMAL_SUPPORTED_VERSION_FOR_IGNORE_UNMAPPED = Version.V_2_11_0; + private static final Map minimalRequiredVersionMap = new HashMap() { + { + put("ignore_unmapped", MINIMAL_SUPPORTED_VERSION_FOR_IGNORE_UNMAPPED); + } + }; + /** * Determines the size of a file on disk in kilobytes * @@ -195,4 +204,12 @@ public static Map getParametersAtLoading(SpaceType spaceType, KN return Collections.unmodifiableMap(loadParameters); } + + public static boolean isClusterOnOrAfterMinRequiredVersion(String key) { + Version minimalRequiredVersion = minimalRequiredVersionMap.get(key); + if (minimalRequiredVersion == null) { + return false; + } + return KNNClusterUtil.instance().getClusterMinVersion().onOrAfter(minimalRequiredVersion); + } } diff --git a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java index 4d039422e..10c652627 100644 --- a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java +++ b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java @@ -7,11 +7,9 @@ import lombok.extern.log4j.Log4j2; import org.apache.lucene.search.MatchNoDocsQuery; -import org.opensearch.Version; import org.opensearch.core.common.Strings; import org.opensearch.index.mapper.NumberFieldMapper; import org.opensearch.index.query.QueryBuilder; -import org.opensearch.knn.index.KNNClusterUtil; import org.opensearch.knn.index.KNNMethodContext; import org.opensearch.knn.index.VectorDataType; import org.opensearch.knn.index.mapper.KNNVectorFieldMapper; @@ -31,11 +29,10 @@ import org.opensearch.index.query.QueryShardContext; import java.io.IOException; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Objects; +import static org.opensearch.knn.index.IndexUtil.isClusterOnOrAfterMinRequiredVersion; import static org.opensearch.knn.index.mapper.KNNVectorFieldMapperUtil.validateByteVectorValue; /** @@ -62,12 +59,6 @@ public class KNNQueryBuilder extends AbstractQueryBuilder { private int k = 0; private QueryBuilder filter; private boolean ignoreUnmapped = false; - private static final Version MINIMAL_SUPPORTED_VERSION_FOR_IGNORE_UNMAPPED = Version.V_2_11_0; - private static final Map minimalRequiredVersionMap = new HashMap() { - { - put("ignore_unmapped", MINIMAL_SUPPORTED_VERSION_FOR_IGNORE_UNMAPPED); - } - }; /** * Constructs a new knn query @@ -363,12 +354,4 @@ protected int doHashCode() { public String getWriteableName() { return NAME; } - - private static boolean isClusterOnOrAfterMinRequiredVersion(String key) { - Version minimalRequiredVersion = minimalRequiredVersionMap.get(key); - if (minimalRequiredVersion == null) { - return false; - } - return KNNClusterUtil.instance().getClusterMinVersion().onOrAfter(minimalRequiredVersion); - } }