Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing unused fetch sub phase processor initialization during fetch… #12503

Merged
merged 6 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Add a system property to configure YamlParser codepoint limits ([#12298](https://github.com/opensearch-project/OpenSearch/pull/12298))
- Prevent read beyond slice boundary in ByteArrayIndexInput ([#10481](https://github.com/opensearch-project/OpenSearch/issues/10481))
- Fix the "highlight.max_analyzer_offset" request parameter with "plain" highlighter ([#10919](https://github.com/opensearch-project/OpenSearch/pull/10919))
- Prevent unnecessary fetch sub phase processor initialization during fetch phase execution ([#12503](https://github.com/opensearch-project/OpenSearch/pull/12503))
- Warn about deprecated and ignored index.mapper.dynamic index setting ([#11193](https://github.com/opensearch-project/OpenSearch/pull/11193))
- Fix `terms` query on `float` field when `doc_values` are turned off by reverting back to `FloatPoint` from `FloatField` ([#12499](https://github.com/opensearch-project/OpenSearch/pull/12499))
- Fix get task API does not refresh resource stats ([#11531](https://github.com/opensearch-project/OpenSearch/pull/11531))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ public boolean includeNamedQueriesScore() {
return searchContext.includeNamedQueriesScore();
}

public boolean hasInnerHits() {
return searchContext.hasInnerHits();
}

/**
* Configuration for returning inner hits
*/
Expand All @@ -213,6 +217,10 @@ public FetchFieldsContext fetchFieldsContext() {
return searchContext.fetchFieldsContext();
}

public boolean hasScriptFields() {
return searchContext.hasScriptFields();
}

/**
* Configuration for script fields
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ public String getName() {
return name;
}

@Override
public boolean hasInnerHits() {
return childInnerHits != null;
}

@Override
public InnerHitsContext innerHits() {
return childInnerHits;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public InnerHitsPhase(FetchPhase fetchPhase) {

@Override
public FetchSubPhaseProcessor getProcessor(FetchContext searchContext) {
if (searchContext.innerHits() == null) {
if (searchContext.hasInnerHits() == false) {
return null;
}
Map<String, InnerHitsContext.InnerHitSubContext> innerHits = searchContext.innerHits().getInnerHits();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public final class ScriptFieldsPhase implements FetchSubPhase {

@Override
public FetchSubPhaseProcessor getProcessor(FetchContext context) {
if (context.scriptFields() == null) {
if (context.hasScriptFields() == false) {
return null;
}
List<ScriptFieldsContext.ScriptField> scriptFields = context.scriptFields().fields();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ public final void close() {

public abstract void highlight(SearchHighlightContext highlight);

public boolean hasInnerHits() {
reta marked this conversation as resolved.
Show resolved Hide resolved
return innerHitsContext != null;
}

public InnerHitsContext innerHits() {
if (innerHitsContext == null) {
innerHitsContext = new InnerHitsContext();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.search.fetch.subphase;

import org.opensearch.index.query.QueryShardContext;
import org.opensearch.search.fetch.FetchContext;
import org.opensearch.search.internal.SearchContext;
import org.opensearch.search.lookup.SearchLookup;
import org.opensearch.test.OpenSearchTestCase;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class InnerHitsPhaseTests extends OpenSearchTestCase {

/*
Returns mock search context reused across test methods
*/
private SearchContext getMockSearchContext(final boolean hasInnerHits) {
final QueryShardContext queryShardContext = mock(QueryShardContext.class);
when(queryShardContext.newFetchLookup()).thenReturn(mock(SearchLookup.class));

final SearchContext searchContext = mock(SearchContext.class);
when(searchContext.hasInnerHits()).thenReturn(hasInnerHits);
when(searchContext.getQueryShardContext()).thenReturn(queryShardContext);

return searchContext;
}

/*
Validates that InnerHitsPhase processor is not initialized when no inner hits
*/
public void testInnerHitsNull() {
assertNull(new InnerHitsPhase(null).getProcessor(new FetchContext(getMockSearchContext(false))));
}

/*
Validates that InnerHitsPhase processor is initialized when inner hits are present
*/
public void testInnerHitsNonNull() {
final SearchContext searchContext = getMockSearchContext(true);
when(searchContext.innerHits()).thenReturn(new InnerHitsContext());

assertNotNull(new InnerHitsPhase(null).getProcessor(new FetchContext(searchContext)));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.search.fetch.subphase;

import org.opensearch.index.query.QueryShardContext;
import org.opensearch.search.fetch.FetchContext;
import org.opensearch.search.internal.SearchContext;
import org.opensearch.search.lookup.SearchLookup;
import org.opensearch.test.OpenSearchTestCase;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class ScriptFieldsPhaseTests extends OpenSearchTestCase {

/*
Returns mock search context reused across test methods
*/
private SearchContext getMockSearchContext(final boolean hasScriptFields) {
final QueryShardContext queryShardContext = mock(QueryShardContext.class);
when(queryShardContext.newFetchLookup()).thenReturn(mock(SearchLookup.class));

final SearchContext searchContext = mock(SearchContext.class);
when(searchContext.hasScriptFields()).thenReturn(hasScriptFields);
when(searchContext.getQueryShardContext()).thenReturn(queryShardContext);

return searchContext;
}

/*
Validates that ScriptFieldsPhase processor is not initialized when no script fields
*/
public void testScriptFieldsNull() {
assertNull(new ScriptFieldsPhase().getProcessor(new FetchContext(getMockSearchContext(false))));
}

/*
Validates that ScriptFieldsPhase processor is initialized when script fields are present
*/
public void testScriptFieldsNonNull() {
final SearchContext searchContext = getMockSearchContext(true);
when(searchContext.scriptFields()).thenReturn(new ScriptFieldsContext());

assertNotNull(new ScriptFieldsPhase().getProcessor(new FetchContext(searchContext)));
}

}
Loading