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

Add method to create empty query collector context with customizable score mode #16660

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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-3.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827))
- Fix compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944))
- Don't over-allocate in HeapBufferedAsyncEntityConsumer in order to consume the response ([#9993](https://github.com/opensearch-project/OpenSearch/pull/9993))
- Add empty query collector context constant with TOP_SCORES score mode ([#16660](https://github.com/opensearch-project/OpenSearch/pull/16660))

### Security

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Locale;

import static org.opensearch.search.profile.query.CollectorResult.REASON_SEARCH_MIN_SCORE;
import static org.opensearch.search.profile.query.CollectorResult.REASON_SEARCH_MULTI;
Expand All @@ -67,38 +68,59 @@
*/
@PublicApi(since = "1.0.0")
public abstract class QueryCollectorContext {
private static final Collector EMPTY_COLLECTOR = new SimpleCollector() {
@Override
public void collect(int doc) {}

@Override
public ScoreMode scoreMode() {
return ScoreMode.COMPLETE_NO_SCORES;
}
};
private static Collector createEmptyCollector(ScoreMode scoreMode) {
martin-gaievski marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please reference the relevant Apache Lucene issue as we've discussed, so it will be clear why we are doing score mode "dance", thanks!

return new SimpleCollector() {
@Override
public void collect(int doc) {}

public static final QueryCollectorContext EMPTY_CONTEXT = new QueryCollectorContext("empty") {
@Override
public ScoreMode scoreMode() {
return scoreMode;
}
};
}

@Override
Collector create(Collector in) throws IOException {
return EMPTY_COLLECTOR;
}
private static final ReduceableSearchResult EMPTY_RESULT = result -> {};

@Override
CollectorManager<?, ReduceableSearchResult> createManager(CollectorManager<?, ReduceableSearchResult> in) throws IOException {
return new CollectorManager<Collector, ReduceableSearchResult>() {
@Override
public Collector newCollector() throws IOException {
return EMPTY_COLLECTOR;
}
private static QueryCollectorContext createEmptyContext(String name, Collector collector) {
return new QueryCollectorContext(name) {
@Override
Collector create(Collector in) {
return collector;
}

@Override
public ReduceableSearchResult reduce(Collection<Collector> collectors) throws IOException {
return result -> {};
}
};
}
};
@Override
CollectorManager<?, ReduceableSearchResult> createManager(CollectorManager<?, ReduceableSearchResult> in) {
return new CollectorManager<>() {

Check warning on line 95 in server/src/main/java/org/opensearch/search/query/QueryCollectorContext.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/query/QueryCollectorContext.java#L95

Added line #L95 was not covered by tests
@Override
public Collector newCollector() {
return collector;

Check warning on line 98 in server/src/main/java/org/opensearch/search/query/QueryCollectorContext.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/query/QueryCollectorContext.java#L98

Added line #L98 was not covered by tests
}

@Override
public ReduceableSearchResult reduce(Collection<Collector> collectors) {
return EMPTY_RESULT;

Check warning on line 103 in server/src/main/java/org/opensearch/search/query/QueryCollectorContext.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/query/QueryCollectorContext.java#L103

Added line #L103 was not covered by tests
}
};

}
};
}

private static String formatContextName(ScoreMode scoreMode) {
return String.format(Locale.ROOT, "empty_with_score_mode_%s", scoreMode.toString().toLowerCase(Locale.ROOT));
}

private static final Collector EMPTY_COLLECTOR = createEmptyCollector(ScoreMode.COMPLETE_NO_SCORES);
public static final QueryCollectorContext EMPTY_CONTEXT = createEmptyContext(
formatContextName(ScoreMode.COMPLETE_NO_SCORES),
martin-gaievski marked this conversation as resolved.
Show resolved Hide resolved
createEmptyCollector(ScoreMode.COMPLETE_NO_SCORES)
);
public static final QueryCollectorContext EMPTY_CONTEXT_TOP_SCORES_SCORE_MODE = createEmptyContext(
formatContextName(ScoreMode.TOP_SCORES),
createEmptyCollector(ScoreMode.TOP_SCORES)
);

private String profilerName;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1197,6 +1197,16 @@ private void createTimeoutCheckerThenWaitThenRun(
verifyNoMoreInteractions(mockedSearchContext);
}

public void testEmptyQueryCollectorContextAndDifferentScoreModes() throws Exception {
QueryCollectorContext topScoresContext = QueryCollectorContext.EMPTY_CONTEXT_TOP_SCORES_SCORE_MODE;

// Verify score mode
assertEquals(org.apache.lucene.search.ScoreMode.TOP_SCORES, topScoresContext.create(null).scoreMode());

// Verify it's a different instance than empty context
assertNotEquals(QueryCollectorContext.EMPTY_CONTEXT, topScoresContext);
}

private static class TestSearchContextWithRewriteAndCancellation extends TestSearchContext {

private TestSearchContextWithRewriteAndCancellation(
Expand Down
Loading