Skip to content

Commit 4155cfe

Browse files
committed
Remove redundant tests
Signed-off-by: Atri Sharma <atri.jiit@gmail.com>
1 parent 87352b7 commit 4155cfe

File tree

2 files changed

+0
-122
lines changed

2 files changed

+0
-122
lines changed

server/src/main/java/org/opensearch/index/query/BoolQueryBuilder.java

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -554,57 +554,4 @@ private boolean checkAllDocsHaveOneValue(List<LeafReaderContext> contexts, Strin
554554
}
555555
return true;
556556
}
557-
558-
// Note: The must-to-filter optimization logic has been moved to MustToFilterRewriter in the query rewriting infrastructure
559-
// The following methods are kept for reference but are no longer used:
560-
/*
561-
private boolean rewriteMustClausesToFilter(BoolQueryBuilder newBuilder, QueryRewriteContext queryRewriteContext) {
562-
// If we have must clauses which return the same score for all matching documents, like numeric term queries or ranges,
563-
// moving them from must clauses to filter clauses improves performance in some cases.
564-
// This works because it can let Lucene use MaxScoreCache to skip non-competitive docs.
565-
boolean changed = false;
566-
Set<QueryBuilder> mustClausesToMove = new HashSet<>();
567-
568-
QueryShardContext shardContext;
569-
if (queryRewriteContext == null) {
570-
shardContext = null;
571-
} else {
572-
shardContext = queryRewriteContext.convertToShardContext(); // can still be null
573-
}
574-
575-
for (QueryBuilder clause : mustClauses) {
576-
if (isClauseIrrelevantToScoring(clause, shardContext)) {
577-
mustClausesToMove.add(clause);
578-
changed = true;
579-
}
580-
}
581-
582-
newBuilder.mustClauses.removeAll(mustClausesToMove);
583-
newBuilder.filterClauses.addAll(mustClausesToMove);
584-
return changed;
585-
}
586-
587-
private boolean isClauseIrrelevantToScoring(QueryBuilder clause, QueryShardContext context) {
588-
// This is an incomplete list of clauses this might apply for; it can be expanded in future.
589-
590-
// If a clause is purely numeric, for example a date range, its score is unimportant as
591-
// it'll be the same for all returned docs
592-
if (clause instanceof RangeQueryBuilder) return true;
593-
if (clause instanceof GeoBoundingBoxQueryBuilder) return true;
594-
595-
// Further optimizations depend on knowing whether the field is numeric.
596-
// QueryBuilder.doRewrite() is called several times in the search flow, and the shard context telling us this
597-
// is only available the last time, when it's called from SearchService.executeQueryPhase().
598-
// Skip moving these clauses if we don't have the shard context.
599-
if (context == null) return false;
600-
if (!(clause instanceof WithFieldName wfn)) return false;
601-
MappedFieldType fieldType = context.fieldMapper(wfn.fieldName());
602-
if (!(fieldType instanceof NumberFieldMapper.NumberFieldType)) return false;
603-
604-
if (clause instanceof MatchQueryBuilder) return true;
605-
if (clause instanceof TermQueryBuilder) return true;
606-
if (clause instanceof TermsQueryBuilder) return true;
607-
return false;
608-
}
609-
*/
610557
}

server/src/test/java/org/opensearch/index/query/BoolQueryBuilderTests.java

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@
7373
import static org.hamcrest.CoreMatchers.equalTo;
7474
import static org.hamcrest.CoreMatchers.instanceOf;
7575
import static org.mockito.Mockito.mock;
76-
import static org.mockito.Mockito.when;
7776

7877
public class BoolQueryBuilderTests extends AbstractQueryTestCase<BoolQueryBuilder> {
7978
@Override
@@ -584,74 +583,6 @@ public void testMustNotRewriteDisabledWithoutExactlyOneValuePerDoc() throws Exce
584583
IOUtils.close(w, reader, dir);
585584
}
586585

587-
// Note: The must-to-filter optimization has been moved to MustToFilterRewriter in the query rewriting infrastructure
588-
// This test is kept but modified to verify that the optimization no longer happens at this level
589-
public void testMustClausesRewritten() throws Exception {
590-
BoolQueryBuilder qb = new BoolQueryBuilder();
591-
592-
// Previously these would be moved, but now they stay in must
593-
QueryBuilder intTermQuery = new TermQueryBuilder(INT_FIELD_NAME, 200);
594-
QueryBuilder rangeQuery = new RangeQueryBuilder(INT_FIELD_NAME).gt(10).lt(20);
595-
QueryBuilder rangeQueryWithBoost = new RangeQueryBuilder(DATE_FIELD_NAME).gt(10).lt(20).boost(2);
596-
QueryBuilder intTermsQuery = new TermsQueryBuilder(INT_FIELD_NAME, new int[] { 1, 4, 100 });
597-
QueryBuilder boundingBoxQuery = new GeoBoundingBoxQueryBuilder(GEO_POINT_FIELD_NAME);
598-
QueryBuilder doubleMatchQuery = new MatchQueryBuilder(DOUBLE_FIELD_NAME, 5.5);
599-
600-
// Text queries
601-
QueryBuilder textTermQuery = new TermQueryBuilder(TEXT_FIELD_NAME, "bar");
602-
QueryBuilder textTermsQuery = new TermsQueryBuilder(TEXT_FIELD_NAME, "foo", "bar");
603-
QueryBuilder textMatchQuery = new MatchQueryBuilder(TEXT_FIELD_NAME, "baz");
604-
605-
qb.must(intTermQuery);
606-
qb.must(rangeQuery);
607-
qb.must(rangeQueryWithBoost);
608-
qb.must(intTermsQuery);
609-
qb.must(boundingBoxQuery);
610-
qb.must(doubleMatchQuery);
611-
612-
qb.must(textTermQuery);
613-
qb.must(textTermsQuery);
614-
qb.must(textMatchQuery);
615-
616-
BoolQueryBuilder rewritten = (BoolQueryBuilder) Rewriteable.rewrite(qb, createShardContext());
617-
618-
// Verify that the must-to-filter optimization no longer happens at this level
619-
// All clauses should remain in must
620-
for (QueryBuilder clause : List.of(
621-
intTermQuery,
622-
rangeQuery,
623-
rangeQueryWithBoost,
624-
intTermsQuery,
625-
boundingBoxQuery,
626-
doubleMatchQuery,
627-
textTermQuery,
628-
textTermsQuery,
629-
textMatchQuery
630-
)) {
631-
assertTrue(rewritten.must().contains(clause));
632-
assertFalse(rewritten.filter().contains(clause));
633-
}
634-
635-
// Same behavior with null context - all clauses remain in must
636-
QueryRewriteContext nullContext = mock(QueryRewriteContext.class);
637-
when(nullContext.convertToShardContext()).thenReturn(null);
638-
rewritten = (BoolQueryBuilder) Rewriteable.rewrite(qb, nullContext);
639-
for (QueryBuilder clause : List.of(
640-
rangeQuery,
641-
rangeQueryWithBoost,
642-
boundingBoxQuery,
643-
textTermQuery,
644-
textTermsQuery,
645-
textMatchQuery,
646-
intTermQuery,
647-
intTermsQuery,
648-
doubleMatchQuery
649-
)) {
650-
assertTrue(rewritten.must().contains(clause));
651-
assertFalse(rewritten.filter().contains(clause));
652-
}
653-
}
654-
655586
private QueryBuilder getRangeQueryBuilder(String fieldName, Integer lower, Integer upper, boolean includeLower, boolean includeUpper) {
656587
RangeQueryBuilder rq = new RangeQueryBuilder(fieldName);
657588
if (lower != null) {

0 commit comments

Comments
 (0)