[GitHub] [lucene] jpountz commented on a change in pull request #672: LUCENE-10418: Optimize `Query#rewrite` in the non-scoring case.

[GitBox <gi...@apache.org>]

jpountz commented on a change in pull request #672:
URL: https://github.com/apache/lucene/pull/672#discussion_r820132595



##########
File path: lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java
##########
@@ -191,51 +191,55 @@ boolean isPureDisjunction() {
     return clauses.iterator();
   }
 
-  private BooleanQuery rewriteNoScoring() {
-    boolean keepShould =
+  BooleanQuery rewriteNoScoring() {
+    boolean actuallyRewritten = false;
+    BooleanQuery.Builder newQuery =
+        new BooleanQuery.Builder().setMinimumNumberShouldMatch(getMinimumNumberShouldMatch());
+
+    final boolean keepShould =
         getMinimumNumberShouldMatch() > 0
             || (clauseSets.get(Occur.MUST).size() + clauseSets.get(Occur.FILTER).size() == 0);
 
-    if (clauseSets.get(Occur.MUST).size() == 0 && keepShould) {
-      return this;
-    }
-    BooleanQuery.Builder newQuery = new BooleanQuery.Builder();
-
-    newQuery.setMinimumNumberShouldMatch(getMinimumNumberShouldMatch());
     for (BooleanClause clause : clauses) {
-      switch (clause.getOccur()) {
-        case MUST:
-          {
-            newQuery.add(clause.getQuery(), Occur.FILTER);
-            break;
-          }
-        case SHOULD:
-          {
-            if (keepShould) {
-              newQuery.add(clause);
-            }
-            break;
-          }
-        case FILTER:
-        case MUST_NOT:
-        default:
-          {
-            newQuery.add(clause);
-          }
+      Query query = clause.getQuery();
+      Query rewritten = ConstantScoreQuery.rewriteNoScoring(query);
+      BooleanClause.Occur occur = clause.getOccur();
+      if (occur == Occur.SHOULD && keepShould == false) {
+        // ignore clause
+        actuallyRewritten = true;
+      } else if (occur == Occur.MUST) {
+        // replace MUST clauses with FILTER clauses
+        newQuery.add(rewritten, Occur.FILTER);
+        actuallyRewritten = true;
+      } else if (query != rewritten) {
+        newQuery.add(rewritten, occur);
+        actuallyRewritten = true;
+      } else {
+        newQuery.add(clause);
       }
     }
 
+    if (actuallyRewritten == false) {
+      return this;
+    }
+
     return newQuery.build();
   }
 
   @Override
   public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost)
       throws IOException {
-    BooleanQuery query = this;
     if (scoreMode.needsScores() == false) {
-      query = rewriteNoScoring();
+      Query rewritten = rewriteNoScoring();
+      if (this != rewritten) {
+        // Pass it back to IndexSearcher#rewrite, which might find new opportunities for rewriting

Review comment:
       Sorry Mike I had missed this comment. I removed this code after Robert's comment but my reasoning here was that in `IndexSearcher#rewrite`, queries get rewritten until the operation is idempotent. So if we introduce another place that rewrites, we would need to call `IndexSearcher#rewrite` on the rewritten query again to simplify it as much as possible given that the new rewrite might have opened new opportunities for simplifications.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org