diff --git a/CHANGELOG.md b/CHANGELOG.md index be18f313e51be..fbb6e45c48188 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add dynamic setting allowing size > 0 requests to be cached in the request cache ([#16483](https://github.com/opensearch-project/OpenSearch/pull/16483)) - Make IndexStoreListener a pluggable interface ([#16583](https://github.com/opensearch-project/OpenSearch/pull/16583)) - Add new configuration setting `synonym_analyzer`, to the `synonym` and `synonym_graph` filters, enabling the specification of a custom analyzer for reading the synonym file ([#16488](https://github.com/opensearch-project/OpenSearch/pull/16488)). +- Add a flag in QueryShardContext to differentiate inner hit query ([#16600](https://github.com/opensearch-project/OpenSearch/pull/16600)) ### Dependencies - Bump `com.azure:azure-storage-common` from 12.25.1 to 12.27.1 ([#16521](https://github.com/opensearch-project/OpenSearch/pull/16521)) diff --git a/modules/search-pipeline-common/src/internalClusterTest/java/org/opensearch/search/pipeline/common/SearchPipelineCommonIT.java b/modules/search-pipeline-common/src/internalClusterTest/java/org/opensearch/search/pipeline/common/SearchPipelineCommonIT.java index b8b0798812df1..35d748bc2d06e 100644 --- a/modules/search-pipeline-common/src/internalClusterTest/java/org/opensearch/search/pipeline/common/SearchPipelineCommonIT.java +++ b/modules/search-pipeline-common/src/internalClusterTest/java/org/opensearch/search/pipeline/common/SearchPipelineCommonIT.java @@ -8,41 +8,187 @@ package org.opensearch.search.pipeline.common; -import org.opensearch.action.admin.indices.delete.DeleteIndexRequest; import org.opensearch.action.admin.indices.refresh.RefreshRequest; import org.opensearch.action.admin.indices.refresh.RefreshResponse; +import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequest; import org.opensearch.action.index.IndexRequest; import org.opensearch.action.index.IndexResponse; import org.opensearch.action.search.DeleteSearchPipelineRequest; +import org.opensearch.action.search.GetSearchPipelineRequest; +import org.opensearch.action.search.GetSearchPipelineResponse; import org.opensearch.action.search.PutSearchPipelineRequest; import org.opensearch.action.search.SearchRequest; import org.opensearch.action.search.SearchResponse; import org.opensearch.action.support.master.AcknowledgedResponse; +import org.opensearch.common.settings.Settings; import org.opensearch.core.common.bytes.BytesArray; +import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.MediaTypeRegistry; import org.opensearch.index.query.MatchAllQueryBuilder; +import org.opensearch.ingest.PipelineConfiguration; import org.opensearch.plugins.Plugin; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.test.OpenSearchIntegTestCase; +import org.junit.After; +import org.junit.Before; import java.util.Collection; +import java.util.HashMap; import java.util.List; import java.util.Map; @OpenSearchIntegTestCase.SuiteScopeTestCase public class SearchPipelineCommonIT extends OpenSearchIntegTestCase { + private static final String TEST_INDEX = "myindex"; + private static final String PIPELINE_NAME = "test_pipeline"; + @Override protected Collection> nodePlugins() { return List.of(SearchPipelineCommonModulePlugin.class); } + @Before + public void setup() throws Exception { + createIndex(TEST_INDEX); + + IndexRequest doc1 = new IndexRequest(TEST_INDEX).id("doc1").source(Map.of("field", "value")); + IndexRequest doc2 = new IndexRequest(TEST_INDEX).id("doc2").source(Map.of("field", "something else")); + + IndexResponse ir = client().index(doc1).actionGet(); + assertSame(RestStatus.CREATED, ir.status()); + ir = client().index(doc2).actionGet(); + assertSame(RestStatus.CREATED, ir.status()); + + RefreshResponse refRsp = client().admin().indices().refresh(new RefreshRequest(TEST_INDEX)).actionGet(); + assertSame(RestStatus.OK, refRsp.getStatus()); + } + + @After + public void cleanup() throws Exception { + internalCluster().wipeIndices(TEST_INDEX); + } + public void testFilterQuery() { // Create a pipeline with a filter_query processor. - String pipelineName = "foo"; + createPipeline(); + + // Search without the pipeline. Should see both documents. + SearchRequest req = new SearchRequest(TEST_INDEX).source(new SearchSourceBuilder().query(new MatchAllQueryBuilder())); + SearchResponse rsp = client().search(req).actionGet(); + assertEquals(2, rsp.getHits().getTotalHits().value); + + // Search with the pipeline. Should only see document with "field":"value". + req.pipeline(PIPELINE_NAME); + rsp = client().search(req).actionGet(); + assertEquals(1, rsp.getHits().getTotalHits().value); + + // Clean up. + deletePipeline(); + } + + public void testSearchWithTemporaryPipeline() throws Exception { + + // Search without the pipeline. Should see both documents. + SearchRequest req = new SearchRequest(TEST_INDEX).source(new SearchSourceBuilder().query(new MatchAllQueryBuilder())); + SearchResponse rsp = client().search(req).actionGet(); + assertEquals(2, rsp.getHits().getTotalHits().value); + + // Search with temporary pipeline + Map pipelineSourceMap = new HashMap<>(); + Map requestProcessorConfig = new HashMap<>(); + + Map filterQuery = new HashMap<>(); + filterQuery.put("query", Map.of("term", Map.of("field", "value"))); + requestProcessorConfig.put("filter_query", filterQuery); + pipelineSourceMap.put("request_processors", List.of(requestProcessorConfig)); + + req = new SearchRequest(TEST_INDEX).source( + new SearchSourceBuilder().query(new MatchAllQueryBuilder()).searchPipelineSource(pipelineSourceMap) + ); + + SearchResponse rspWithTempPipeline = client().search(req).actionGet(); + assertEquals(1, rspWithTempPipeline.getHits().getTotalHits().value); + } + + public void testSearchWithDefaultPipeline() throws Exception { + // Create pipeline + createPipeline(); + + // Search without the pipeline. Should see both documents. + SearchRequest req = new SearchRequest(TEST_INDEX).source(new SearchSourceBuilder().query(new MatchAllQueryBuilder())); + SearchResponse rsp = client().search(req).actionGet(); + assertEquals(2, rsp.getHits().getTotalHits().value); + + // Set pipeline as default for the index + UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest(TEST_INDEX); + updateSettingsRequest.settings(Settings.builder().put("index.search.default_pipeline", PIPELINE_NAME)); + AcknowledgedResponse updateSettingsResponse = client().admin().indices().updateSettings(updateSettingsRequest).actionGet(); + assertTrue(updateSettingsResponse.isAcknowledged()); + + // Search with the default pipeline. Should only see document with "field":"value". + rsp = client().search(req).actionGet(); + assertEquals(1, rsp.getHits().getTotalHits().value); + + // Clean up: Remove default pipeline setting + updateSettingsRequest = new UpdateSettingsRequest(TEST_INDEX); + updateSettingsRequest.settings(Settings.builder().putNull("index.search.default_pipeline")); + updateSettingsResponse = client().admin().indices().updateSettings(updateSettingsRequest).actionGet(); + assertTrue(updateSettingsResponse.isAcknowledged()); + + // Clean up. + deletePipeline(); + } + + public void testUpdateSearchPipeline() throws Exception { + // Create initial pipeline + createPipeline(); + + // Verify initial pipeline + SearchRequest req = new SearchRequest(TEST_INDEX).source(new SearchSourceBuilder().query(new MatchAllQueryBuilder())); + req.pipeline(PIPELINE_NAME); + SearchResponse initialRsp = client().search(req).actionGet(); + assertEquals(1, initialRsp.getHits().getTotalHits().value); + + BytesReference pipelineConfig = new BytesArray( + "{" + + "\"description\": \"Updated pipeline\"," + + "\"request_processors\": [" + + "{" + + "\"filter_query\" : {" + + "\"query\": {" + + "\"term\" : {" + + "\"field\" : \"something else\"" + + "}" + + "}" + + "}" + + "}" + + "]" + + "}" + ); + + PipelineConfiguration pipeline = new PipelineConfiguration(PIPELINE_NAME, pipelineConfig, MediaTypeRegistry.JSON); + + // Update pipeline + PutSearchPipelineRequest updateRequest = new PutSearchPipelineRequest(pipeline.getId(), pipelineConfig, MediaTypeRegistry.JSON); + AcknowledgedResponse ackRsp = client().admin().cluster().putSearchPipeline(updateRequest).actionGet(); + assertTrue(ackRsp.isAcknowledged()); + + // Verify pipeline description + GetSearchPipelineResponse getPipelineResponse = client().admin() + .cluster() + .getSearchPipeline(new GetSearchPipelineRequest(PIPELINE_NAME)) + .actionGet(); + assertEquals(PIPELINE_NAME, getPipelineResponse.pipelines().get(0).getId()); + assertEquals(pipeline.getConfigAsMap(), getPipelineResponse.pipelines().get(0).getConfigAsMap()); + // Clean up. + deletePipeline(); + } + + private void createPipeline() { PutSearchPipelineRequest putSearchPipelineRequest = new PutSearchPipelineRequest( - pipelineName, + PIPELINE_NAME, new BytesArray( "{" + "\"request_processors\": [" @@ -62,35 +208,13 @@ public void testFilterQuery() { ); AcknowledgedResponse ackRsp = client().admin().cluster().putSearchPipeline(putSearchPipelineRequest).actionGet(); assertTrue(ackRsp.isAcknowledged()); + } - // Index some documents. - String indexName = "myindex"; - IndexRequest doc1 = new IndexRequest(indexName).id("doc1").source(Map.of("field", "value")); - IndexRequest doc2 = new IndexRequest(indexName).id("doc2").source(Map.of("field", "something else")); - - IndexResponse ir = client().index(doc1).actionGet(); - assertSame(RestStatus.CREATED, ir.status()); - ir = client().index(doc2).actionGet(); - assertSame(RestStatus.CREATED, ir.status()); - - // Refresh so the documents are visible to search. - RefreshResponse refRsp = client().admin().indices().refresh(new RefreshRequest(indexName)).actionGet(); - assertSame(RestStatus.OK, refRsp.getStatus()); - - // Search without the pipeline. Should see both documents. - SearchRequest req = new SearchRequest(indexName).source(new SearchSourceBuilder().query(new MatchAllQueryBuilder())); - SearchResponse rsp = client().search(req).actionGet(); - assertEquals(2, rsp.getHits().getTotalHits().value); - - // Search with the pipeline. Should only see document with "field":"value". - req.pipeline(pipelineName); - rsp = client().search(req).actionGet(); - assertEquals(1, rsp.getHits().getTotalHits().value); - - // Clean up. - ackRsp = client().admin().cluster().deleteSearchPipeline(new DeleteSearchPipelineRequest(pipelineName)).actionGet(); - assertTrue(ackRsp.isAcknowledged()); - ackRsp = client().admin().indices().delete(new DeleteIndexRequest(indexName)).actionGet(); + private void deletePipeline() { + AcknowledgedResponse ackRsp = client().admin() + .cluster() + .deleteSearchPipeline(new DeleteSearchPipelineRequest(PIPELINE_NAME)) + .actionGet(); assertTrue(ackRsp.isAcknowledged()); } } diff --git a/server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java index 5908882472ce7..ec7e62035a82f 100644 --- a/server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java @@ -413,6 +413,7 @@ protected void doBuild(SearchContext parentSearchContext, InnerHitsContext inner try { queryShardContext.setParentFilter(parentFilter); queryShardContext.nestedScope().nextLevel(nestedObjectMapper); + queryShardContext.setInnerHitQuery(true); try { NestedInnerHitSubContext nestedInnerHits = new NestedInnerHitSubContext( name, @@ -427,6 +428,7 @@ protected void doBuild(SearchContext parentSearchContext, InnerHitsContext inner } } finally { queryShardContext.setParentFilter(previousParentFilter); + queryShardContext.setInnerHitQuery(false); } } } diff --git a/server/src/main/java/org/opensearch/index/query/QueryShardContext.java b/server/src/main/java/org/opensearch/index/query/QueryShardContext.java index bccead2b029d0..d717f10b17d9c 100644 --- a/server/src/main/java/org/opensearch/index/query/QueryShardContext.java +++ b/server/src/main/java/org/opensearch/index/query/QueryShardContext.java @@ -126,6 +126,7 @@ public class QueryShardContext extends QueryRewriteContext { private BitSetProducer parentFilter; private DerivedFieldResolver derivedFieldResolver; private boolean keywordIndexOrDocValuesEnabled; + private boolean isInnerHitQuery; public QueryShardContext( int shardId, @@ -727,4 +728,12 @@ public BitSetProducer getParentFilter() { public void setParentFilter(BitSetProducer parentFilter) { this.parentFilter = parentFilter; } + + public boolean isInnerHitQuery() { + return isInnerHitQuery; + } + + public void setInnerHitQuery(boolean isInnerHitQuery) { + this.isInnerHitQuery = isInnerHitQuery; + } } diff --git a/server/src/test/java/org/opensearch/index/query/NestedQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/NestedQueryBuilderTests.java index 351011eb1b812..c367d123402d4 100644 --- a/server/src/test/java/org/opensearch/index/query/NestedQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/query/NestedQueryBuilderTests.java @@ -335,6 +335,9 @@ public void testParentFilterFromInlineLeafInnerHitsNestedQuery() throws Exceptio if (context.getParentFilter() == null) { throw new Exception("Expect parent filter to be non-null"); } + if (context.isInnerHitQuery() == false) { + throw new Exception("Expect it to be inner hit query"); + } return invoke.callRealMethod(); }); NestedQueryBuilder query = new NestedQueryBuilder("nested1", innerQueryBuilder, ScoreMode.None); @@ -345,6 +348,7 @@ public void testParentFilterFromInlineLeafInnerHitsNestedQuery() throws Exceptio assertThat(innerHitBuilders.size(), Matchers.equalTo(1)); assertTrue(innerHitBuilders.containsKey(leafInnerHits.getName())); assertNull(queryShardContext.getParentFilter()); + assertFalse(queryShardContext.isInnerHitQuery()); innerHitBuilders.get(leafInnerHits.getName()).build(searchContext, innerHitsContext); assertNull(queryShardContext.getParentFilter()); verify(innerQueryBuilder).toQuery(queryShardContext);