From c1b12fdfb46a65eeaf4d8ebcee7913c84f906686 Mon Sep 17 00:00:00 2001 From: Mrudhul Guda Date: Sat, 27 Jan 2024 19:19:54 +0530 Subject: [PATCH 1/8] FIX: UOE While Exists query building for nested search_as_you_type Signed-off-by: Mrudhul Guda --- .../mapper/SearchAsYouTypeFieldMapper.java | 5 --- .../SearchAsYouTypeFieldMapperTests.java | 45 +++++++++++++------ .../index/mapper/MapperServiceTestCase.java | 2 + 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapper.java b/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapper.java index 366e848416328..5f3556a353295 100644 --- a/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapper.java @@ -454,11 +454,6 @@ public String typeName() { public String toString() { return super.toString() + ",prefixChars=" + minChars + ":" + maxChars; } - - @Override - public Query existsQuery(QueryShardContext context) { - throw new UnsupportedOperationException(); - } } static final class PrefixFieldMapper extends FieldMapper { diff --git a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java index b5f687ce34d4b..92e1776f9fbec 100644 --- a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java @@ -41,15 +41,8 @@ import org.apache.lucene.queries.spans.FieldMaskingSpanQuery; import org.apache.lucene.queries.spans.SpanNearQuery; import org.apache.lucene.queries.spans.SpanTermQuery; -import org.apache.lucene.search.BooleanClause; -import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.ConstantScoreQuery; -import org.apache.lucene.search.DisjunctionMaxQuery; -import org.apache.lucene.search.MatchNoDocsQuery; -import org.apache.lucene.search.MultiPhraseQuery; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.SynonymQuery; -import org.apache.lucene.search.TermQuery; +import org.apache.lucene.queryparser.classic.ParseException; +import org.apache.lucene.search.*; import org.opensearch.common.lucene.search.MultiPhrasePrefixQuery; import org.opensearch.core.common.Strings; import org.opensearch.core.xcontent.MediaTypeRegistry; @@ -64,10 +57,8 @@ import org.opensearch.index.mapper.SearchAsYouTypeFieldMapper.SearchAsYouTypeFieldType; import org.opensearch.index.mapper.SearchAsYouTypeFieldMapper.ShingleFieldMapper; import org.opensearch.index.mapper.SearchAsYouTypeFieldMapper.ShingleFieldType; -import org.opensearch.index.query.MatchPhrasePrefixQueryBuilder; -import org.opensearch.index.query.MatchPhraseQueryBuilder; -import org.opensearch.index.query.MultiMatchQueryBuilder; -import org.opensearch.index.query.QueryShardContext; +import org.opensearch.index.query.*; +import org.opensearch.index.search.QueryStringQueryParser; import org.opensearch.plugins.Plugin; import java.io.IOException; @@ -541,6 +532,34 @@ public void testMatchPhrase() throws IOException { } } + public void testNestedExistsQuery() throws IOException { + MapperService mapperService = createMapperService(mapping(b -> { + b.startObject("field"); + { + b.field("type", "object"); + b.startObject("properties"); + { + b.startObject("nested_field"); + { + b.field("type", "search_as_you_type"); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + })); + QueryShardContext queryShardContext = createQueryShardContext(mapperService); + Query actual = new QueryStringQueryBuilder("field:*").toQuery(queryShardContext); + Query expected = new ConstantScoreQuery(new BooleanQuery.Builder() + .add(new NormsFieldExistsQuery("field.nested_field"), BooleanClause.Occur.SHOULD) + .add(new NormsFieldExistsQuery("field.nested_field._3gram"), BooleanClause.Occur.SHOULD) + .add(new NormsFieldExistsQuery("field.nested_field._2gram"), BooleanClause.Occur.SHOULD) + .add(new TermQuery(new Term("_field_names", "field.nested_field._index_prefix")), BooleanClause.Occur.SHOULD) + .build()); + assertThat(actual, equalTo(expected)); + } + private static BooleanQuery buildBoolPrefixQuery(String shingleFieldName, String prefixFieldName, List terms) { final BooleanQuery.Builder builder = new BooleanQuery.Builder(); for (int i = 0; i < terms.size() - 1; i++) { diff --git a/test/framework/src/main/java/org/opensearch/index/mapper/MapperServiceTestCase.java b/test/framework/src/main/java/org/opensearch/index/mapper/MapperServiceTestCase.java index ac78a0d1936ea..dcb0eae1f0cb9 100644 --- a/test/framework/src/main/java/org/opensearch/index/mapper/MapperServiceTestCase.java +++ b/test/framework/src/main/java/org/opensearch/index/mapper/MapperServiceTestCase.java @@ -247,6 +247,8 @@ protected QueryShardContext createQueryShardContext(MapperService mapperService) when(queryShardContext.getSearchQuoteAnalyzer(any())).thenCallRealMethod(); when(queryShardContext.getSearchAnalyzer(any())).thenCallRealMethod(); when(queryShardContext.getIndexSettings()).thenReturn(mapperService.getIndexSettings()); + when(queryShardContext.getObjectMapper(anyString())).thenAnswer( + inv -> mapperService.getObjectMapper(inv.getArguments()[0].toString())); when(queryShardContext.simpleMatchToIndexNames(any())).thenAnswer( inv -> mapperService.simpleMatchToFullName(inv.getArguments()[0].toString()) ); From c17b106e33c8f8eaab54226273c9785ce174afb3 Mon Sep 17 00:00:00 2001 From: Mrudhul Guda Date: Sat, 27 Jan 2024 22:52:53 +0530 Subject: [PATCH 2/8] Added fix in the change log Signed-off-by: Mrudhul Guda --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa31819ffae97..8f99900357ce7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Update supported version for max_shard_size parameter in Shrink API ([#11439](https://github.com/opensearch-project/OpenSearch/pull/11439)) - Fix typo in API annotation check message ([11836](https://github.com/opensearch-project/OpenSearch/pull/11836)) - Update supported version for must_exist parameter in update aliases API ([#11872](https://github.com/opensearch-project/OpenSearch/pull/11872)) +- Fix UOE While building Exists query for nested search_as_you_type field ([#12048](https://github.com/opensearch-project/OpenSearch/pull/12048)) ### Security From eadc8fb93bcd7fe367ac5c8d6ae5afb4a0f968e2 Mon Sep 17 00:00:00 2001 From: Mrudhul Guda Date: Thu, 1 Feb 2024 22:32:15 +0530 Subject: [PATCH 3/8] Refactored based on Requested changes in PR Signed-off-by: Mrudhul Guda --- .../SearchAsYouTypeFieldMapperTests.java | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java index 92e1776f9fbec..9576dbbde3cab 100644 --- a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java @@ -41,8 +41,16 @@ import org.apache.lucene.queries.spans.FieldMaskingSpanQuery; import org.apache.lucene.queries.spans.SpanNearQuery; import org.apache.lucene.queries.spans.SpanTermQuery; -import org.apache.lucene.queryparser.classic.ParseException; -import org.apache.lucene.search.*; +import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.DisjunctionMaxQuery; +import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.MultiPhraseQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.SynonymQuery; +import org.apache.lucene.search.TermQuery; +import org.apache.lucene.search.NormsFieldExistsQuery; import org.opensearch.common.lucene.search.MultiPhrasePrefixQuery; import org.opensearch.core.common.Strings; import org.opensearch.core.xcontent.MediaTypeRegistry; @@ -57,8 +65,11 @@ import org.opensearch.index.mapper.SearchAsYouTypeFieldMapper.SearchAsYouTypeFieldType; import org.opensearch.index.mapper.SearchAsYouTypeFieldMapper.ShingleFieldMapper; import org.opensearch.index.mapper.SearchAsYouTypeFieldMapper.ShingleFieldType; -import org.opensearch.index.query.*; -import org.opensearch.index.search.QueryStringQueryParser; +import org.opensearch.index.query.MatchPhrasePrefixQueryBuilder; +import org.opensearch.index.query.MatchPhraseQueryBuilder; +import org.opensearch.index.query.MultiMatchQueryBuilder; +import org.opensearch.index.query.QueryShardContext; +import org.opensearch.index.query.QueryStringQueryBuilder; import org.opensearch.plugins.Plugin; import java.io.IOException; @@ -557,7 +568,7 @@ public void testNestedExistsQuery() throws IOException { .add(new NormsFieldExistsQuery("field.nested_field._2gram"), BooleanClause.Occur.SHOULD) .add(new TermQuery(new Term("_field_names", "field.nested_field._index_prefix")), BooleanClause.Occur.SHOULD) .build()); - assertThat(actual, equalTo(expected)); + assertEquals(actual, expected); } private static BooleanQuery buildBoolPrefixQuery(String shingleFieldName, String prefixFieldName, List terms) { From ba4c8b6d065807245ea0f5bb7dae135e573d5a1d Mon Sep 17 00:00:00 2001 From: Mrudhul Guda Date: Thu, 1 Feb 2024 22:37:58 +0530 Subject: [PATCH 4/8] Refactored based on Requested changes in PR Swapped AssertEquals Signed-off-by: Mrudhul Guda --- .../index/mapper/SearchAsYouTypeFieldMapperTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java index 9576dbbde3cab..1755f2fb7fef3 100644 --- a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java @@ -568,7 +568,7 @@ public void testNestedExistsQuery() throws IOException { .add(new NormsFieldExistsQuery("field.nested_field._2gram"), BooleanClause.Occur.SHOULD) .add(new TermQuery(new Term("_field_names", "field.nested_field._index_prefix")), BooleanClause.Occur.SHOULD) .build()); - assertEquals(actual, expected); + assertEquals(expected, actual); } private static BooleanQuery buildBoolPrefixQuery(String shingleFieldName, String prefixFieldName, List terms) { From 7f9d56e5568975341ca74def0f7ad343c39a0a60 Mon Sep 17 00:00:00 2001 From: Mrudhul Guda Date: Thu, 1 Feb 2024 23:52:59 +0530 Subject: [PATCH 5/8] Applied Spot less check Signed-off-by: Mrudhul Guda --- .../mapper/SearchAsYouTypeFieldMapperTests.java | 15 ++++++++------- .../index/mapper/MapperServiceTestCase.java | 3 ++- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java index 1755f2fb7fef3..750289684628d 100644 --- a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java @@ -47,10 +47,10 @@ import org.apache.lucene.search.DisjunctionMaxQuery; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.MultiPhraseQuery; +import org.apache.lucene.search.NormsFieldExistsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.SynonymQuery; import org.apache.lucene.search.TermQuery; -import org.apache.lucene.search.NormsFieldExistsQuery; import org.opensearch.common.lucene.search.MultiPhrasePrefixQuery; import org.opensearch.core.common.Strings; import org.opensearch.core.xcontent.MediaTypeRegistry; @@ -562,12 +562,13 @@ public void testNestedExistsQuery() throws IOException { })); QueryShardContext queryShardContext = createQueryShardContext(mapperService); Query actual = new QueryStringQueryBuilder("field:*").toQuery(queryShardContext); - Query expected = new ConstantScoreQuery(new BooleanQuery.Builder() - .add(new NormsFieldExistsQuery("field.nested_field"), BooleanClause.Occur.SHOULD) - .add(new NormsFieldExistsQuery("field.nested_field._3gram"), BooleanClause.Occur.SHOULD) - .add(new NormsFieldExistsQuery("field.nested_field._2gram"), BooleanClause.Occur.SHOULD) - .add(new TermQuery(new Term("_field_names", "field.nested_field._index_prefix")), BooleanClause.Occur.SHOULD) - .build()); + Query expected = new ConstantScoreQuery( + new BooleanQuery.Builder().add(new NormsFieldExistsQuery("field.nested_field"), BooleanClause.Occur.SHOULD) + .add(new NormsFieldExistsQuery("field.nested_field._3gram"), BooleanClause.Occur.SHOULD) + .add(new NormsFieldExistsQuery("field.nested_field._2gram"), BooleanClause.Occur.SHOULD) + .add(new TermQuery(new Term("_field_names", "field.nested_field._index_prefix")), BooleanClause.Occur.SHOULD) + .build() + ); assertEquals(expected, actual); } diff --git a/test/framework/src/main/java/org/opensearch/index/mapper/MapperServiceTestCase.java b/test/framework/src/main/java/org/opensearch/index/mapper/MapperServiceTestCase.java index dcb0eae1f0cb9..9c5d420425ca8 100644 --- a/test/framework/src/main/java/org/opensearch/index/mapper/MapperServiceTestCase.java +++ b/test/framework/src/main/java/org/opensearch/index/mapper/MapperServiceTestCase.java @@ -248,7 +248,8 @@ protected QueryShardContext createQueryShardContext(MapperService mapperService) when(queryShardContext.getSearchAnalyzer(any())).thenCallRealMethod(); when(queryShardContext.getIndexSettings()).thenReturn(mapperService.getIndexSettings()); when(queryShardContext.getObjectMapper(anyString())).thenAnswer( - inv -> mapperService.getObjectMapper(inv.getArguments()[0].toString())); + inv -> mapperService.getObjectMapper(inv.getArguments()[0].toString()) + ); when(queryShardContext.simpleMatchToIndexNames(any())).thenAnswer( inv -> mapperService.simpleMatchToFullName(inv.getArguments()[0].toString()) ); From 0ff598a3644a6707d5c42676a037cd13e4560368 Mon Sep 17 00:00:00 2001 From: Mrudhul Guda Date: Thu, 8 Feb 2024 17:51:35 +0530 Subject: [PATCH 6/8] Resolved latest conflicts in changelog file Signed-off-by: Mrudhul Guda --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a5f60209115d..02dd4b362e256 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix typo in API annotation check message ([11836](https://github.com/opensearch-project/OpenSearch/pull/11836)) - Update supported version for must_exist parameter in update aliases API ([#11872](https://github.com/opensearch-project/OpenSearch/pull/11872)) - [Bug] Check phase name before SearchRequestOperationsListener onPhaseStart ([#12035](https://github.com/opensearch-project/OpenSearch/pull/12035)) +- Fix Span operation names generated from RestActions ([#12005](https://github.com/opensearch-project/OpenSearch/pull/12005)) - Fix UOE While building Exists query for nested search_as_you_type field ([#12048](https://github.com/opensearch-project/OpenSearch/pull/12048)) ### Security From fa4264a2d99e62b395f86e73551637b7a9ebc9b9 Mon Sep 17 00:00:00 2001 From: Mrudhul Guda Date: Wed, 13 Mar 2024 23:50:23 +0530 Subject: [PATCH 7/8] Added fix in exists query builder to skip queries generation for virtual fields Signed-off-by: Mrudhul Guda --- .../opensearch/index/mapper/SearchAsYouTypeFieldMapper.java | 5 +++++ .../index/mapper/SearchAsYouTypeFieldMapperTests.java | 6 ++---- .../java/org/opensearch/index/query/ExistsQueryBuilder.java | 5 +++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapper.java b/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapper.java index 5f3556a353295..366e848416328 100644 --- a/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapper.java @@ -454,6 +454,11 @@ public String typeName() { public String toString() { return super.toString() + ",prefixChars=" + minChars + ":" + maxChars; } + + @Override + public Query existsQuery(QueryShardContext context) { + throw new UnsupportedOperationException(); + } } static final class PrefixFieldMapper extends FieldMapper { diff --git a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java index 750289684628d..ae2d52fc92576 100644 --- a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java @@ -563,10 +563,8 @@ public void testNestedExistsQuery() throws IOException { QueryShardContext queryShardContext = createQueryShardContext(mapperService); Query actual = new QueryStringQueryBuilder("field:*").toQuery(queryShardContext); Query expected = new ConstantScoreQuery( - new BooleanQuery.Builder().add(new NormsFieldExistsQuery("field.nested_field"), BooleanClause.Occur.SHOULD) - .add(new NormsFieldExistsQuery("field.nested_field._3gram"), BooleanClause.Occur.SHOULD) - .add(new NormsFieldExistsQuery("field.nested_field._2gram"), BooleanClause.Occur.SHOULD) - .add(new TermQuery(new Term("_field_names", "field.nested_field._index_prefix")), BooleanClause.Occur.SHOULD) + new BooleanQuery.Builder() + .add(new NormsFieldExistsQuery("field.nested_field"), BooleanClause.Occur.SHOULD) .build() ); assertEquals(expected, actual); diff --git a/server/src/main/java/org/opensearch/index/query/ExistsQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/ExistsQueryBuilder.java index 7fd83d5753512..3011a48fbb296 100644 --- a/server/src/main/java/org/opensearch/index/query/ExistsQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/ExistsQueryBuilder.java @@ -201,6 +201,11 @@ private static Query newObjectFieldExistsQuery(QueryShardContext context, String BooleanQuery.Builder booleanQuery = new BooleanQuery.Builder(); Collection fields = context.simpleMatchToIndexNames(objField + ".*"); for (String field : fields) { + int dotPos = field.lastIndexOf('.'); + if (dotPos > 0 && field.charAt(dotPos + 1) == '_') { + // This is a subfield (e.g. prefix) of a complex field type. Skip it. + continue; + } Query existsQuery = context.getMapperService().fieldType(field).existsQuery(context); booleanQuery.add(existsQuery, Occur.SHOULD); } From e0a9a0897e82c6c9684c1a7bd55c734dd4ff3daf Mon Sep 17 00:00:00 2001 From: Mrudhul Guda Date: Thu, 14 Mar 2024 00:10:13 +0530 Subject: [PATCH 8/8] Applied spotless check for latest changes Signed-off-by: Mrudhul Guda --- .../index/mapper/SearchAsYouTypeFieldMapperTests.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java index ae2d52fc92576..f55ad2e9d659c 100644 --- a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/SearchAsYouTypeFieldMapperTests.java @@ -563,9 +563,7 @@ public void testNestedExistsQuery() throws IOException { QueryShardContext queryShardContext = createQueryShardContext(mapperService); Query actual = new QueryStringQueryBuilder("field:*").toQuery(queryShardContext); Query expected = new ConstantScoreQuery( - new BooleanQuery.Builder() - .add(new NormsFieldExistsQuery("field.nested_field"), BooleanClause.Occur.SHOULD) - .build() + new BooleanQuery.Builder().add(new NormsFieldExistsQuery("field.nested_field"), BooleanClause.Occur.SHOULD).build() ); assertEquals(expected, actual); }