Skip to content

Commit 0524afd

Browse files
atrisatris
committed
Update per comments
Signed-off-by: Atri Sharma <atri.jiit@gmail.com>
1 parent 03bf50f commit 0524afd

14 files changed

+26
-135
lines changed

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -400,11 +400,6 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws
400400
return any.get();
401401
}
402402

403-
// Note: must-not-to-should optimization is now handled by MustNotToShouldRewriter in the query rewriting infrastructure
404-
// changed |= rewriteMustNotRangeClausesToShould(newBuilder, queryRewriteContext);
405-
// Note: must-to-filter optimization is now handled by MustToFilterRewriter in the query rewriting infrastructure
406-
// changed |= rewriteMustClausesToFilter(newBuilder, queryRewriteContext);
407-
408403
if (changed) {
409404
newBuilder.adjustPureNegative = adjustPureNegative;
410405
if (minimumShouldMatch != null) {

‎server/src/main/java/org/opensearch/search/SearchService.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,12 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
284284
Property.NodeScope
285285
);
286286

287+
/**
288+
* Controls the threshold for the number of term queries on the same field that triggers
289+
* the TermsMergingRewriter to combine them into a single terms query. For example,
290+
* if set to 16 (default), when 16 or more term queries target the same field within
291+
* a boolean clause, they will be merged into a single terms query for better performance.
292+
*/
287293
public static final Setting<Integer> QUERY_REWRITING_TERMS_THRESHOLD_SETTING = Setting.intSetting(
288294
"search.query_rewriting.terms_threshold",
289295
16,

‎server/src/main/java/org/opensearch/search/query/QueryRewriterRegistry.java‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212
import org.apache.logging.log4j.Logger;
1313
import org.opensearch.index.query.QueryBuilder;
1414
import org.opensearch.index.query.QueryShardContext;
15+
import org.opensearch.search.query.rewriters.BooleanFlatteningRewriter;
16+
import org.opensearch.search.query.rewriters.MatchAllRemovalRewriter;
17+
import org.opensearch.search.query.rewriters.MustNotToShouldRewriter;
18+
import org.opensearch.search.query.rewriters.MustToFilterRewriter;
19+
import org.opensearch.search.query.rewriters.TermsMergingRewriter;
1520

1621
import java.util.ArrayList;
1722
import java.util.Comparator;
@@ -25,22 +30,6 @@
2530
public final class QueryRewriterRegistry {
2631

2732
private static final Logger logger = LogManager.getLogger(QueryRewriterRegistry.class);
28-
private static volatile List<QueryRewriter> rewriters;
29-
30-
static {
31-
initializeDefaultRewriters();
32-
}
33-
34-
private static void initializeDefaultRewriters() {
35-
List<QueryRewriter> defaultRewriters = new ArrayList<>();
36-
defaultRewriters.add(new BooleanFlatteningRewriter());
37-
defaultRewriters.add(new MustToFilterRewriter());
38-
defaultRewriters.add(new MustNotToShouldRewriter());
39-
defaultRewriters.add(new TermsMergingRewriter());
40-
defaultRewriters.add(new MatchAllRemovalRewriter());
41-
defaultRewriters.sort(Comparator.comparingInt(QueryRewriter::priority));
42-
rewriters = defaultRewriters;
43-
}
4433

4534
private QueryRewriterRegistry() {}
4635

server/src/main/java/org/opensearch/search/query/BooleanFlatteningRewriter.java renamed to server/src/main/java/org/opensearch/search/query/rewriters/BooleanFlatteningRewriter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,12 @@
66
* compatible open source license.
77
*/
88

9-
package org.opensearch.search.query;
9+
package org.opensearch.search.query.rewriters;
1010

1111
import org.opensearch.index.query.BoolQueryBuilder;
1212
import org.opensearch.index.query.QueryBuilder;
1313
import org.opensearch.index.query.QueryShardContext;
14+
import org.opensearch.search.query.QueryRewriter;
1415

1516
import java.util.ArrayList;
1617
import java.util.List;

server/src/main/java/org/opensearch/search/query/MatchAllRemovalRewriter.java renamed to server/src/main/java/org/opensearch/search/query/rewriters/MatchAllRemovalRewriter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66
* compatible open source license.
77
*/
88

9-
package org.opensearch.search.query;
9+
package org.opensearch.search.query.rewriters;
1010

1111
import org.opensearch.index.query.BoolQueryBuilder;
1212
import org.opensearch.index.query.ConstantScoreQueryBuilder;
1313
import org.opensearch.index.query.MatchAllQueryBuilder;
1414
import org.opensearch.index.query.QueryBuilder;
1515
import org.opensearch.index.query.QueryShardContext;
16+
import org.opensearch.search.query.QueryRewriter;
1617

1718
import java.util.List;
1819

server/src/main/java/org/opensearch/search/query/MustNotToShouldRewriter.java renamed to server/src/main/java/org/opensearch/search/query/rewriters/MustNotToShouldRewriter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* compatible open source license.
77
*/
88

9-
package org.opensearch.search.query;
9+
package org.opensearch.search.query.rewriters;
1010

1111
import org.apache.lucene.index.LeafReaderContext;
1212
import org.apache.lucene.index.PointValues;
@@ -15,6 +15,7 @@
1515
import org.opensearch.index.query.QueryBuilder;
1616
import org.opensearch.index.query.QueryShardContext;
1717
import org.opensearch.index.query.WithFieldName;
18+
import org.opensearch.search.query.QueryRewriter;
1819

1920
import java.io.IOException;
2021
import java.util.ArrayList;

server/src/main/java/org/opensearch/search/query/MustToFilterRewriter.java renamed to server/src/main/java/org/opensearch/search/query/rewriters/MustToFilterRewriter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* compatible open source license.
77
*/
88

9-
package org.opensearch.search.query;
9+
package org.opensearch.search.query.rewriters;
1010

1111
import org.opensearch.index.mapper.MappedFieldType;
1212
import org.opensearch.index.mapper.NumberFieldMapper;
@@ -19,6 +19,7 @@
1919
import org.opensearch.index.query.TermQueryBuilder;
2020
import org.opensearch.index.query.TermsQueryBuilder;
2121
import org.opensearch.index.query.WithFieldName;
22+
import org.opensearch.search.query.QueryRewriter;
2223

2324
import java.util.ArrayList;
2425
import java.util.List;

server/src/main/java/org/opensearch/search/query/TermsMergingRewriter.java renamed to server/src/main/java/org/opensearch/search/query/rewriters/TermsMergingRewriter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66
* compatible open source license.
77
*/
88

9-
package org.opensearch.search.query;
9+
package org.opensearch.search.query.rewriters;
1010

1111
import org.opensearch.index.query.BoolQueryBuilder;
1212
import org.opensearch.index.query.QueryBuilder;
1313
import org.opensearch.index.query.QueryShardContext;
1414
import org.opensearch.index.query.TermQueryBuilder;
1515
import org.opensearch.index.query.TermsQueryBuilder;
16+
import org.opensearch.search.query.QueryRewriter;
1617

1718
import java.util.ArrayList;
1819
import java.util.HashMap;

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

Lines changed: 0 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -517,71 +517,6 @@ public void testVisit() {
517517

518518
}
519519

520-
// Note: Must-not-to-should optimization is now handled by MustNotToShouldRewriter in the query rewriting infrastructure
521-
// This test is disabled as the functionality has been moved out of BoolQueryBuilder
522-
/*
523-
public void testOneMustNotRangeRewritten() throws Exception {
524-
int from = 10;
525-
int to = 20;
526-
Directory dir = newDirectory();
527-
IndexWriter w = new IndexWriter(dir, newIndexWriterConfig(new StandardAnalyzer()));
528-
addDocument(w, INT_FIELD_NAME, 1);
529-
DirectoryReader reader = DirectoryReader.open(w);
530-
IndexSearcher searcher = getIndexSearcher(reader);
531-
532-
for (boolean includeLower : new boolean[] { true, false }) {
533-
for (boolean includeUpper : new boolean[] { true, false }) {
534-
BoolQueryBuilder qb = new BoolQueryBuilder();
535-
QueryBuilder rq = getRangeQueryBuilder(INT_FIELD_NAME, from, to, includeLower, includeUpper);
536-
qb.mustNot(rq);
537-
538-
BoolQueryBuilder rewritten = (BoolQueryBuilder) Rewriteable.rewrite(qb, createShardContext(searcher));
539-
assertFalse(rewritten.mustNot().contains(rq));
540-
541-
QueryBuilder expectedLowerQuery = getRangeQueryBuilder(INT_FIELD_NAME, null, from, false, !includeLower);
542-
QueryBuilder expectedUpperQuery = getRangeQueryBuilder(INT_FIELD_NAME, to, null, !includeUpper, true);
543-
assertEquals(1, rewritten.must().size());
544-
545-
BoolQueryBuilder nestedBoolQuery = (BoolQueryBuilder) rewritten.must().get(0);
546-
assertEquals(2, nestedBoolQuery.should().size());
547-
assertEquals("1", nestedBoolQuery.minimumShouldMatch());
548-
assertTrue(nestedBoolQuery.should().contains(expectedLowerQuery));
549-
assertTrue(nestedBoolQuery.should().contains(expectedUpperQuery));
550-
}
551-
}
552-
IOUtils.close(w, reader, dir);
553-
}
554-
*/
555-
556-
// Note: Must-not-to-should optimization is now handled by MustNotToShouldRewriter in the query rewriting infrastructure
557-
// This test is disabled as the functionality has been moved out of BoolQueryBuilder
558-
/*
559-
public void testOneSingleEndedMustNotRangeRewritten() throws Exception {
560-
// Test a must_not range query with only one endpoint is rewritten correctly
561-
int from = 10;
562-
Directory dir = newDirectory();
563-
IndexWriter w = new IndexWriter(dir, newIndexWriterConfig(new StandardAnalyzer()));
564-
addDocument(w, INT_FIELD_NAME, 1);
565-
DirectoryReader reader = DirectoryReader.open(w);
566-
IndexSearcher searcher = getIndexSearcher(reader);
567-
568-
BoolQueryBuilder qb = new BoolQueryBuilder();
569-
QueryBuilder rq = getRangeQueryBuilder(INT_FIELD_NAME, from, null, false, false);
570-
qb.mustNot(rq);
571-
BoolQueryBuilder rewritten = (BoolQueryBuilder) Rewriteable.rewrite(qb, createShardContext(searcher));
572-
assertFalse(rewritten.mustNot().contains(rq));
573-
574-
QueryBuilder expectedQuery = getRangeQueryBuilder(INT_FIELD_NAME, null, from, false, true);
575-
assertEquals(1, rewritten.must().size());
576-
BoolQueryBuilder nestedBoolQuery = (BoolQueryBuilder) rewritten.must().get(0);
577-
assertEquals(1, nestedBoolQuery.should().size());
578-
assertTrue(nestedBoolQuery.should().contains(expectedQuery));
579-
assertEquals("1", nestedBoolQuery.minimumShouldMatch());
580-
581-
IOUtils.close(w, reader, dir);
582-
}
583-
*/
584-
585520
public void testMultipleComplementAwareOnSameFieldNotRewritten() throws Exception {
586521
Directory dir = newDirectory();
587522
IndexWriter w = new IndexWriter(dir, newIndexWriterConfig(new StandardAnalyzer()));
@@ -649,45 +584,6 @@ public void testMustNotRewriteDisabledWithoutExactlyOneValuePerDoc() throws Exce
649584
IOUtils.close(w, reader, dir);
650585
}
651586

652-
// Note: Must-not-to-should optimization is now handled by MustNotToShouldRewriter in the query rewriting infrastructure
653-
// This test is disabled as the functionality has been moved out of BoolQueryBuilder
654-
/*
655-
public void testOneMustNotNumericMatchQueryRewritten() throws Exception {
656-
Directory dir = newDirectory();
657-
IndexWriter w = new IndexWriter(dir, newIndexWriterConfig(new StandardAnalyzer()));
658-
addDocument(w, INT_FIELD_NAME, 1);
659-
DirectoryReader reader = DirectoryReader.open(w);
660-
IndexSearcher searcher = getIndexSearcher(reader);
661-
662-
BoolQueryBuilder qb = new BoolQueryBuilder();
663-
int excludedValue = 200;
664-
QueryBuilder matchQuery = new MatchQueryBuilder(INT_FIELD_NAME, excludedValue);
665-
qb.mustNot(matchQuery);
666-
667-
BoolQueryBuilder rewritten = (BoolQueryBuilder) Rewriteable.rewrite(qb, createShardContext(searcher));
668-
assertFalse(rewritten.mustNot().contains(matchQuery));
669-
670-
QueryBuilder expectedLowerQuery = getRangeQueryBuilder(INT_FIELD_NAME, null, excludedValue, true, false);
671-
QueryBuilder expectedUpperQuery = getRangeQueryBuilder(INT_FIELD_NAME, excludedValue, null, false, true);
672-
assertEquals(1, rewritten.must().size());
673-
674-
BoolQueryBuilder nestedBoolQuery = (BoolQueryBuilder) rewritten.must().get(0);
675-
assertEquals(2, nestedBoolQuery.should().size());
676-
assertEquals("1", nestedBoolQuery.minimumShouldMatch());
677-
assertTrue(nestedBoolQuery.should().contains(expectedLowerQuery));
678-
assertTrue(nestedBoolQuery.should().contains(expectedUpperQuery));
679-
680-
// When the QueryShardContext is null, we should not rewrite any match queries as we can't confirm if they're on numeric fields.
681-
QueryRewriteContext nullContext = mock(QueryRewriteContext.class);
682-
when(nullContext.convertToShardContext()).thenReturn(null);
683-
BoolQueryBuilder rewrittenNoContext = (BoolQueryBuilder) Rewriteable.rewrite(qb, nullContext);
684-
assertTrue(rewrittenNoContext.mustNot().contains(matchQuery));
685-
assertTrue(rewrittenNoContext.should().isEmpty());
686-
687-
IOUtils.close(w, reader, dir);
688-
}
689-
*/
690-
691587
// Note: The must-to-filter optimization has been moved to MustToFilterRewriter in the query rewriting infrastructure
692588
// This test is kept but modified to verify that the optimization no longer happens at this level
693589
public void testMustClausesRewritten() throws Exception {

server/src/test/java/org/opensearch/search/query/BooleanFlatteningRewriterTests.java renamed to server/src/test/java/org/opensearch/search/query/rewriters/BooleanFlatteningRewriterTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* compatible open source license.
77
*/
88

9-
package org.opensearch.search.query;
9+
package org.opensearch.search.query.rewriters;
1010

1111
import org.apache.lucene.tests.util.LuceneTestCase.AwaitsFix;
1212
import org.opensearch.index.query.BoolQueryBuilder;

0 commit comments

Comments
 (0)