diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 6fea1351170..fd49a73f68d 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 def245585d9..2e2d81b53de 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 { @Override public void visit(QueryVisitor visitor) { - for (Map.Entry> entry : clauseSets.entrySet()) { - Occur clauseOccur = entry.getKey(); - Collection 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 f1a43105474..27254d8579f 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 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))))))")); } }