From c40cdc63b567a6a5113d3912e4c1f90ff2bd741d Mon Sep 17 00:00:00 2001 From: Heemin Kim Date: Thu, 30 May 2024 07:40:01 -0700 Subject: [PATCH] Revert "Pass parent filter to inner hit query (#13770)" This reverts commit db5240e06dc08e7d7d03432595bb4d93b0e2e32d. --- CHANGELOG.md | 1 - .../index/query/NestedQueryBuilder.java | 36 ++++++------------- .../index/query/NestedQueryBuilderTests.java | 35 ------------------ 3 files changed, 10 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5416005e4706c..782c9d04be0b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,7 +45,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix get field mapping API returns 404 error in mixed cluster with multiple versions ([#13624](https://github.com/opensearch-project/OpenSearch/pull/13624)) - Allow clearing `remote_store.compatibility_mode` setting ([#13646](https://github.com/opensearch-project/OpenSearch/pull/13646)) - Fix ReplicaShardBatchAllocator to batch shards without duplicates ([#13710](https://github.com/opensearch-project/OpenSearch/pull/13710)) -- Pass parent filter to inner hit query ([#13770](https://github.com/opensearch-project/OpenSearch/pull/13770)) ### Security 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 b5ba79632b622..3f97b3918a126 100644 --- a/server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java @@ -401,32 +401,16 @@ protected void doBuild(SearchContext parentSearchContext, InnerHitsContext inner } } String name = innerHitBuilder.getName() != null ? innerHitBuilder.getName() : nestedObjectMapper.fullPath(); - ObjectMapper parentObjectMapper = queryShardContext.nestedScope().getObjectMapper(); - BitSetProducer parentFilter; - if (parentObjectMapper == null) { - parentFilter = queryShardContext.bitsetFilter(Queries.newNonNestedFilter()); - } else { - parentFilter = queryShardContext.bitsetFilter(parentObjectMapper.nestedTypeFilter()); - } - BitSetProducer previousParentFilter = queryShardContext.getParentFilter(); - try { - queryShardContext.setParentFilter(parentFilter); - queryShardContext.nestedScope().nextLevel(nestedObjectMapper); - try { - NestedInnerHitSubContext nestedInnerHits = new NestedInnerHitSubContext( - name, - parentSearchContext, - parentObjectMapper, - nestedObjectMapper - ); - setupInnerHitsContext(queryShardContext, nestedInnerHits); - innerHitsContext.addInnerHitDefinition(nestedInnerHits); - } finally { - queryShardContext.nestedScope().previousLevel(); - } - } finally { - queryShardContext.setParentFilter(previousParentFilter); - } + ObjectMapper parentObjectMapper = queryShardContext.nestedScope().nextLevel(nestedObjectMapper); + NestedInnerHitSubContext nestedInnerHits = new NestedInnerHitSubContext( + name, + parentSearchContext, + parentObjectMapper, + nestedObjectMapper + ); + setupInnerHitsContext(queryShardContext, nestedInnerHits); + queryShardContext.nestedScope().previousLevel(); + innerHitsContext.addInnerHitDefinition(nestedInnerHits); } } 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 096acd23429bd..29efd64e5c751 100644 --- a/server/src/test/java/org/opensearch/index/query/NestedQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/query/NestedQueryBuilderTests.java @@ -311,41 +311,6 @@ public void testInlineLeafInnerHitsNestedQuery() throws Exception { assertThat(innerHitBuilders.get(leafInnerHits.getName()), Matchers.notNullValue()); } - public void testParentFilterFromInlineLeafInnerHitsNestedQuery() throws Exception { - QueryShardContext queryShardContext = createShardContext(); - SearchContext searchContext = mock(SearchContext.class); - when(searchContext.getQueryShardContext()).thenReturn(queryShardContext); - - MapperService mapperService = mock(MapperService.class); - IndexSettings settings = new IndexSettings(newIndexMeta("index", Settings.EMPTY), Settings.EMPTY); - when(mapperService.getIndexSettings()).thenReturn(settings); - when(searchContext.mapperService()).thenReturn(mapperService); - - InnerHitBuilder leafInnerHits = randomNestedInnerHits(); - leafInnerHits.setScriptFields(null); - leafInnerHits.setHighlightBuilder(null); - - QueryBuilder innerQueryBuilder = spy(new MatchAllQueryBuilder()); - when(innerQueryBuilder.toQuery(queryShardContext)).thenAnswer(invoke -> { - QueryShardContext context = invoke.getArgument(0); - if (context.getParentFilter() == null) { - throw new Exception("Expect parent filter to be non-null"); - } - return invoke.callRealMethod(); - }); - NestedQueryBuilder query = new NestedQueryBuilder("nested1", innerQueryBuilder, ScoreMode.None); - query.innerHit(leafInnerHits); - final Map innerHitBuilders = new HashMap<>(); - final InnerHitsContext innerHitsContext = new InnerHitsContext(); - query.extractInnerHitBuilders(innerHitBuilders); - assertThat(innerHitBuilders.size(), Matchers.equalTo(1)); - assertTrue(innerHitBuilders.containsKey(leafInnerHits.getName())); - assertNull(queryShardContext.getParentFilter()); - innerHitBuilders.get(leafInnerHits.getName()).build(searchContext, innerHitsContext); - assertNull(queryShardContext.getParentFilter()); - verify(innerQueryBuilder).toQuery(queryShardContext); - } - public void testInlineLeafInnerHitsNestedQueryViaBoolQuery() { InnerHitBuilder leafInnerHits = randomNestedInnerHits(); NestedQueryBuilder nestedQueryBuilder = new NestedQueryBuilder("path", new MatchAllQueryBuilder(), ScoreMode.None).innerHit(