You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ro...@apache.org on 2019/10/16 16:26:26 UTC

[lucene-solr] branch master updated: LUCENE-9005: BooleanQuery.visit() pulls subvisitors from a top-level MUST visitor

This is an automated email from the ASF dual-hosted git repository.

romseygeek pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/master by this push:
     new f7711d7  LUCENE-9005: BooleanQuery.visit() pulls subvisitors from a top-level MUST visitor
f7711d7 is described below

commit f7711d712472528b567ab975d0ed677bbd30ac12
Author: Alan Woodward <ro...@apache.org>
AuthorDate: Wed Oct 16 16:03:44 2019 +0100

    LUCENE-9005: BooleanQuery.visit() pulls subvisitors from a top-level MUST visitor
---
 lucene/CHANGES.txt                                  |  6 ++++++
 .../java/org/apache/lucene/search/BooleanQuery.java | 21 +++++++++++++--------
 .../org/apache/lucene/search/TestQueryVisitor.java  | 18 ++++++++++++++++++
 3 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 6fea135..fd49a73 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -217,6 +217,12 @@ Bug Fixes
 * LUCENE-8755: spatial-extras quad and packed quad prefix trees could throw a
   NullPointerException for certain cell edge coordinates (Chongchen Chen, David Smiley)
 
+* LUCENE-9005: BooleanQuery.visit() would pull subVisitors from its parent visitor, rather
+  than from a visitor for its own specific query.  This could cause problems when BQ was
+  nested under another BQ. Instead, we now pull a MUST subvisitor, pass it to any MUST
+  subclauses, and then pull SHOULD, MUST_NOT and FILTER visitors from it rather than from
+  the parent.  (Alan Woodward)
+
 Other
 
 * LUCENE-8778 LUCENE-8911 LUCENE-8957: Define analyzer SPI names as static final fields and document the names in Javadocs.
diff --git a/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java b/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java
index def2455..2e2d81b 100644
--- a/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java
+++ b/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java
@@ -506,14 +506,19 @@ public class BooleanQuery extends Query implements Iterable<BooleanClause> {
 
   @Override
   public void visit(QueryVisitor visitor) {
-    for (Map.Entry<Occur, Collection<Query>> entry : clauseSets.entrySet()) {
-      Occur clauseOccur = entry.getKey();
-      Collection<Query> clauseQueries = entry.getValue();
-
-      if (clauseQueries.size() > 0) {
-        QueryVisitor v = visitor.getSubVisitor(clauseOccur, this);
-        for (Query q : clauseQueries) {
-          q.visit(v);
+    QueryVisitor sub = visitor.getSubVisitor(Occur.MUST, this);
+    for (BooleanClause.Occur occur : clauseSets.keySet()) {
+      if (clauseSets.get(occur).size() > 0) {
+        if (occur == Occur.MUST) {
+          for (Query q : clauseSets.get(occur)) {
+            q.visit(sub);
+          }
+        }
+        else {
+          QueryVisitor v = sub.getSubVisitor(occur, this);
+          for (Query q : clauseSets.get(occur)) {
+            q.visit(v);
+          }
         }
       }
     }
diff --git a/lucene/core/src/test/org/apache/lucene/search/TestQueryVisitor.java b/lucene/core/src/test/org/apache/lucene/search/TestQueryVisitor.java
index f1a4310..27254d8 100644
--- a/lucene/core/src/test/org/apache/lucene/search/TestQueryVisitor.java
+++ b/lucene/core/src/test/org/apache/lucene/search/TestQueryVisitor.java
@@ -328,6 +328,24 @@ public class TestQueryVisitor extends LuceneTestCase {
     minimumTermSet.clear();
     extractor.collectTerms(minimumTermSet);
     assertThat(minimumTermSet, equalTo(expected2));
+
+    BooleanQuery bq = new BooleanQuery.Builder()
+        .add(new BooleanQuery.Builder()
+            .add(new TermQuery(new Term("f", "1")), BooleanClause.Occur.MUST)
+            .add(new TermQuery(new Term("f", "61")), BooleanClause.Occur.MUST)
+            .add(new TermQuery(new Term("f", "211")), BooleanClause.Occur.FILTER)
+            .add(new TermQuery(new Term("f", "5")), BooleanClause.Occur.SHOULD)
+            .build(), BooleanClause.Occur.SHOULD)
+        .add(new PhraseQuery("f", "3333", "44444"), BooleanClause.Occur.SHOULD)
+        .build();
+    QueryNode ex2 = new ConjunctionNode();
+    bq.visit(ex2);
+    Set<Term> expected3 = new HashSet<>(Arrays.asList(new Term("f", "1"), new Term("f", "3333")));
+    minimumTermSet.clear();
+    ex2.collectTerms(minimumTermSet);
+    assertThat(minimumTermSet, equalTo(expected3));
+    ex2.getWeight(); // force sort order
+    assertThat(ex2.toString(), equalTo("AND(AND(OR(AND(TERM(f:3333),TERM(f:44444)),AND(TERM(f:1),TERM(f:61),AND(TERM(f:211))))))"));
   }
 
 }