LUCENE-9005: BooleanQuery.visit() pulls subvisitors from a top-level MUST visitor

This commit is contained in:
Alan Woodward 2019-10-16 16:03:44 +01:00
parent 621461fd1a
commit f7711d7124
3 changed files with 37 additions and 8 deletions

View File

@ -217,6 +217,12 @@ Bug Fixes
* LUCENE-8755: spatial-extras quad and packed quad prefix trees could throw a * LUCENE-8755: spatial-extras quad and packed quad prefix trees could throw a
NullPointerException for certain cell edge coordinates (Chongchen Chen, David Smiley) 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 Other
* LUCENE-8778 LUCENE-8911 LUCENE-8957: Define analyzer SPI names as static final fields and document the names in Javadocs. * LUCENE-8778 LUCENE-8911 LUCENE-8957: Define analyzer SPI names as static final fields and document the names in Javadocs.

View File

@ -506,18 +506,23 @@ public class BooleanQuery extends Query implements Iterable<BooleanClause> {
@Override @Override
public void visit(QueryVisitor visitor) { public void visit(QueryVisitor visitor) {
for (Map.Entry<Occur, Collection<Query>> entry : clauseSets.entrySet()) { QueryVisitor sub = visitor.getSubVisitor(Occur.MUST, this);
Occur clauseOccur = entry.getKey(); for (BooleanClause.Occur occur : clauseSets.keySet()) {
Collection<Query> clauseQueries = entry.getValue(); if (clauseSets.get(occur).size() > 0) {
if (occur == Occur.MUST) {
if (clauseQueries.size() > 0) { for (Query q : clauseSets.get(occur)) {
QueryVisitor v = visitor.getSubVisitor(clauseOccur, this); q.visit(sub);
for (Query q : clauseQueries) { }
}
else {
QueryVisitor v = sub.getSubVisitor(occur, this);
for (Query q : clauseSets.get(occur)) {
q.visit(v); q.visit(v);
} }
} }
} }
} }
}
/** Prints a user-readable version of this query. */ /** Prints a user-readable version of this query. */
@Override @Override

View File

@ -328,6 +328,24 @@ public class TestQueryVisitor extends LuceneTestCase {
minimumTermSet.clear(); minimumTermSet.clear();
extractor.collectTerms(minimumTermSet); extractor.collectTerms(minimumTermSet);
assertThat(minimumTermSet, equalTo(expected2)); 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))))))"));
} }
} }