From 223eaa2bcf74ea3b363abfb161b3771b09f48281 Mon Sep 17 00:00:00 2001 From: Luc Vanlerberghe Date: Thu, 3 Mar 2016 11:52:14 +0100 Subject: [PATCH 1/3] LUCENE-7064: Split MultiPhraseQuery into an immutable class and a Builder --- .../lucene/search/MultiPhraseQuery.java | 147 +++++++++++------- .../org/apache/lucene/util/QueryBuilder.java | 18 +-- .../classic/MultiFieldQueryParser.java | 6 +- .../queryparser/classic/QueryParserBase.java | 9 +- .../builders/MultiPhraseQueryNodeBuilder.java | 6 +- .../builders/SlopQueryNodeBuilder.java | 8 +- .../solr/parser/SolrQueryParserBase.java | 11 +- .../solr/search/ExtendedDismaxQParser.java | 8 +- 8 files changed, 133 insertions(+), 80 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/search/MultiPhraseQuery.java b/lucene/core/src/java/org/apache/lucene/search/MultiPhraseQuery.java index b665388c68c7..e571f00ad216 100644 --- a/lucene/core/src/java/org/apache/lucene/search/MultiPhraseQuery.java +++ b/lucene/core/src/java/org/apache/lucene/search/MultiPhraseQuery.java @@ -37,74 +37,111 @@ import org.apache.lucene.util.PriorityQueue; /** - * A generalized version of {@link PhraseQuery}, with an added - * method {@link #add(Term[])} for adding more than one term at the same position - * that are treated as a disjunction (OR). - * To use this class to search for the phrase "Microsoft app*" first use - * {@link #add(Term)} on the term "microsoft" (assuming lowercase analysis), then + * A generalized version of {@link PhraseQuery}, with the possibility of + * adding more than one term at the same position that are treated as a disjunction (OR). + * To use this class to search for the phrase "Microsoft app*" first create a Builder and use + * {@link Builder#add(Term)} on the term "microsoft" (assuming lowercase analysis), then * find all terms that have "app" as prefix using {@link LeafReader#terms(String)}, * seeking to "app" then iterating and collecting terms until there is no longer - * that prefix, and finally use {@link #add(Term[])} to add them to the query. + * that prefix, and finally use {@link Builder#add(Term[])} to add them. + * {@link Builder#build()} returns the fully constructed (and immutable) MultiPhraseQuery. */ public class MultiPhraseQuery extends Query { - private String field;// becomes non-null on first add() then is unmodified - private final ArrayList termArrays = new ArrayList<>(); - private final ArrayList positions = new ArrayList<>(); - - private int slop = 0; - - /** Sets the phrase slop for this query. - * @see PhraseQuery#getSlop() - */ - public void setSlop(int s) { - if (s < 0) { - throw new IllegalArgumentException("slop value cannot be negative"); + /** A builder for multi-phrase queries */ + public static class Builder { + private String field; // becomes non-null on first add() then is unmodified + private final ArrayList termArrays; + private final ArrayList positions; + private int slop; + + public Builder() { + this.field = null; + this.termArrays = new ArrayList<>(); + this.positions = new ArrayList<>(); + this.slop = 0; + } + + public Builder(MultiPhraseQuery multiPhraseQuery) { + this.field = multiPhraseQuery.field; + this.termArrays = new ArrayList<>(multiPhraseQuery.termArrays); + this.positions = new ArrayList<>(multiPhraseQuery.positions); + this.slop = multiPhraseQuery.slop; + } + + /** Sets the phrase slop for this query. + * @see PhraseQuery#getSlop() + */ + public Builder setSlop(int s) { + if (s < 0) { + throw new IllegalArgumentException("slop value cannot be negative"); + } + slop = s; + + return this; } - slop = s; - } - /** Sets the phrase slop for this query. - * @see PhraseQuery#getSlop() - */ - public int getSlop() { return slop; } + /** Add a single term at the next position in the phrase. + */ + public Builder add(Term term) { return add(new Term[]{term}); } - /** Add a single term at the next position in the phrase. - */ - public void add(Term term) { add(new Term[]{term}); } + /** Add multiple terms at the next position in the phrase. Any of the terms + * may match (a disjunction). + * The array is not copied or mutated, the caller should consider it + * immutable subsequent to calling this method. + */ + public Builder add(Term[] terms) { + int position = 0; + if (positions.size() > 0) + position = positions.get(positions.size() - 1) + 1; - /** Add multiple terms at the next position in the phrase. Any of the terms - * may match (a disjunction). - * The array is not copied or mutated, the caller should consider it - * immutable subsequent to calling this method. - */ - public void add(Term[] terms) { - int position = 0; - if (positions.size() > 0) - position = positions.get(positions.size() - 1) + 1; + return add(terms, position); + } - add(terms, position); - } + /** + * Allows to specify the relative position of terms within the phrase. + * The array is not copied or mutated, the caller should consider it + * immutable subsequent to calling this method. + */ + public Builder add(Term[] terms, int position) { + Objects.requireNonNull(terms, "Term array must not be null"); + if (termArrays.size() == 0) + field = terms[0].field(); - /** - * Allows to specify the relative position of terms within the phrase. - * The array is not copied or mutated, the caller should consider it - * immutable subsequent to calling this method. - */ - public void add(Term[] terms, int position) { - Objects.requireNonNull(terms, "Term array must not be null"); - if (termArrays.size() == 0) - field = terms[0].field(); - - for (Term term : terms) { - if (!term.field().equals(field)) { - throw new IllegalArgumentException( - "All phrase terms must be in the same field (" + field + "): " + term); + for (Term term : terms) { + if (!term.field().equals(field)) { + throw new IllegalArgumentException( + "All phrase terms must be in the same field (" + field + "): " + term); + } } - } - termArrays.add(terms); - positions.add(position); + termArrays.add(terms); + positions.add(position); + + return this; + } + + public MultiPhraseQuery build() { + return new MultiPhraseQuery(field, termArrays, positions, slop); + } + } + + private final String field; + private final ArrayList termArrays; // TODO: Change to Term[][] + private final ArrayList positions; // TODO: Change to int[] + private final int slop; + + private MultiPhraseQuery(String field, ArrayList termArrays, ArrayList positions, int slop) { + // No argument checks here since they are provided by the MultiPhraseQuery.Builder + this.field = field; + this.termArrays = termArrays; + this.positions = positions; + this.slop = slop; } + + /** Sets the phrase slop for this query. + * @see PhraseQuery#getSlop() + */ + public int getSlop() { return slop; } /** * Returns a List of the terms in the multi-phrase. diff --git a/lucene/core/src/java/org/apache/lucene/util/QueryBuilder.java b/lucene/core/src/java/org/apache/lucene/util/QueryBuilder.java index 7acff490c7cd..259335b0bbee 100644 --- a/lucene/core/src/java/org/apache/lucene/util/QueryBuilder.java +++ b/lucene/core/src/java/org/apache/lucene/util/QueryBuilder.java @@ -354,8 +354,8 @@ private Query analyzePhrase(String field, TokenStream stream, int slop) throws I * Creates complex phrase query from the cached tokenstream contents */ private Query analyzeMultiPhrase(String field, TokenStream stream, int slop) throws IOException { - MultiPhraseQuery mpq = newMultiPhraseQuery(); - mpq.setSlop(slop); + MultiPhraseQuery.Builder mpqb = newMultiPhraseQueryBuilder(); + mpqb.setSlop(slop); TermToBytesRefAttribute termAtt = stream.getAttribute(TermToBytesRefAttribute.class); @@ -369,9 +369,9 @@ private Query analyzeMultiPhrase(String field, TokenStream stream, int slop) thr if (positionIncrement > 0 && multiTerms.size() > 0) { if (enablePositionIncrements) { - mpq.add(multiTerms.toArray(new Term[0]), position); + mpqb.add(multiTerms.toArray(new Term[0]), position); } else { - mpq.add(multiTerms.toArray(new Term[0])); + mpqb.add(multiTerms.toArray(new Term[0])); } multiTerms.clear(); } @@ -380,11 +380,11 @@ private Query analyzeMultiPhrase(String field, TokenStream stream, int slop) thr } if (enablePositionIncrements) { - mpq.add(multiTerms.toArray(new Term[0]), position); + mpqb.add(multiTerms.toArray(new Term[0]), position); } else { - mpq.add(multiTerms.toArray(new Term[0])); + mpqb.add(multiTerms.toArray(new Term[0])); } - return mpq; + return mpqb.build(); } /** @@ -424,7 +424,7 @@ protected Query newTermQuery(Term term) { * This is intended for subclasses that wish to customize the generated queries. * @return new MultiPhraseQuery instance */ - protected MultiPhraseQuery newMultiPhraseQuery() { - return new MultiPhraseQuery(); + protected MultiPhraseQuery.Builder newMultiPhraseQueryBuilder() { + return new MultiPhraseQuery.Builder(); } } diff --git a/lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/MultiFieldQueryParser.java b/lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/MultiFieldQueryParser.java index 02d25c409ec9..b9963ec1bd5b 100644 --- a/lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/MultiFieldQueryParser.java +++ b/lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/MultiFieldQueryParser.java @@ -134,7 +134,11 @@ private Query applySlop(Query q, int slop) { } q = builder.build(); } else if (q instanceof MultiPhraseQuery) { - ((MultiPhraseQuery) q).setSlop(slop); + MultiPhraseQuery mpq = (MultiPhraseQuery)q; + + if (slop != mpq.getSlop()) { + q = new MultiPhraseQuery.Builder(mpq).setSlop(slop).build(); + } } return q; } diff --git a/lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/QueryParserBase.java b/lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/QueryParserBase.java index c988b8c09087..d0a371c03582 100644 --- a/lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/QueryParserBase.java +++ b/lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/QueryParserBase.java @@ -502,9 +502,12 @@ protected Query getFieldQuery(String field, String queryText, int slop) builder.add(terms[i], positions[i]); } query = builder.build(); - } - if (query instanceof MultiPhraseQuery) { - ((MultiPhraseQuery) query).setSlop(slop); + } else if (query instanceof MultiPhraseQuery) { + MultiPhraseQuery mpq = (MultiPhraseQuery)query; + + if (slop != mpq.getSlop()) { + query = new MultiPhraseQuery.Builder(mpq).setSlop(slop).build(); + } } return query; diff --git a/lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/builders/MultiPhraseQueryNodeBuilder.java b/lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/builders/MultiPhraseQueryNodeBuilder.java index 6976a04ceda5..35debe96f4e0 100644 --- a/lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/builders/MultiPhraseQueryNodeBuilder.java +++ b/lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/builders/MultiPhraseQueryNodeBuilder.java @@ -43,7 +43,7 @@ public MultiPhraseQueryNodeBuilder() { public MultiPhraseQuery build(QueryNode queryNode) throws QueryNodeException { MultiPhraseQueryNode phraseNode = (MultiPhraseQueryNode) queryNode; - MultiPhraseQuery phraseQuery = new MultiPhraseQuery(); + MultiPhraseQuery.Builder phraseQueryBuilder = new MultiPhraseQuery.Builder(); List children = phraseNode.getChildren(); @@ -70,14 +70,14 @@ public MultiPhraseQuery build(QueryNode queryNode) throws QueryNodeException { for (int positionIncrement : positionTermMap.keySet()) { List termList = positionTermMap.get(positionIncrement); - phraseQuery.add(termList.toArray(new Term[termList.size()]), + phraseQueryBuilder.add(termList.toArray(new Term[termList.size()]), positionIncrement); } } - return phraseQuery; + return phraseQueryBuilder.build(); } diff --git a/lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/builders/SlopQueryNodeBuilder.java b/lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/builders/SlopQueryNodeBuilder.java index e96434694322..77667d9e17d0 100644 --- a/lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/builders/SlopQueryNodeBuilder.java +++ b/lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/builders/SlopQueryNodeBuilder.java @@ -55,7 +55,13 @@ public Query build(QueryNode queryNode) throws QueryNodeException { query = builder.build(); } else { - ((MultiPhraseQuery) query).setSlop(phraseSlopNode.getValue()); + MultiPhraseQuery mpq = (MultiPhraseQuery)query; + + int slop = phraseSlopNode.getValue(); + + if (slop != mpq.getSlop()) { + query = new MultiPhraseQuery.Builder(mpq).setSlop(slop).build(); + } } return query; diff --git a/solr/core/src/java/org/apache/solr/parser/SolrQueryParserBase.java b/solr/core/src/java/org/apache/solr/parser/SolrQueryParserBase.java index 20fee1e854ab..81c0fd726ee2 100644 --- a/solr/core/src/java/org/apache/solr/parser/SolrQueryParserBase.java +++ b/solr/core/src/java/org/apache/solr/parser/SolrQueryParserBase.java @@ -387,7 +387,6 @@ protected Query getFieldQuery(String field, String queryText, int slop) // only set slop of the phrase query was a result of this parser // and not a sub-parser. if (subQParser == null) { - if (query instanceof PhraseQuery) { PhraseQuery pq = (PhraseQuery) query; Term[] terms = pq.getTerms(); @@ -398,11 +397,13 @@ protected Query getFieldQuery(String field, String queryText, int slop) } builder.setSlop(slop); query = builder.build(); + } else if (query instanceof MultiPhraseQuery) { + MultiPhraseQuery mpq = (MultiPhraseQuery)query; + + if (slop != mpq.getSlop()) { + query = new MultiPhraseQuery.Builder(mpq).setSlop(slop).build(); + } } - if (query instanceof MultiPhraseQuery) { - ((MultiPhraseQuery) query).setSlop(slop); - } - } return query; diff --git a/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java b/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java index 467129234433..71dac7255317 100644 --- a/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java +++ b/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java @@ -1235,9 +1235,11 @@ private Query getQuery() { builder.setSlop(slop); query = builder.build(); } else if (query instanceof MultiPhraseQuery) { - MultiPhraseQuery pq = (MultiPhraseQuery)query; - if (minClauseSize > 1 && pq.getTermArrays().size() < minClauseSize) return null; - ((MultiPhraseQuery)query).setSlop(slop); + MultiPhraseQuery mpq = (MultiPhraseQuery)query; + if (minClauseSize > 1 && mpq.getTermArrays().size() < minClauseSize) return null; + if (slop != mpq.getSlop()) { + query = new MultiPhraseQuery.Builder(mpq).setSlop(slop).build(); + } } else if (minClauseSize > 1) { // if it's not a type of phrase query, it doesn't meet the minClauseSize requirements return null; From 1b4c0ed77fe8b22c7a04b2879b34514b0ac05d0d Mon Sep 17 00:00:00 2001 From: Luc Vanlerberghe Date: Thu, 3 Mar 2016 14:02:51 +0100 Subject: [PATCH 2/3] LUCENE-7064: Updated tests --- .../search/TestComplexExplanations.java | 10 +- .../lucene/search/TestMultiPhraseQuery.java | 190 ++++++++++-------- .../lucene/search/TestPhrasePrefixQuery.java | 16 +- .../lucene/search/TestPositionIncrement.java | 6 +- .../lucene/search/TestSimpleExplanations.java | 52 ++--- .../search/TestSimpleSearchEquivalence.java | 16 +- .../lucene/search/TestSloppyPhraseQuery2.java | 10 +- .../apache/lucene/util/TestQueryBuilder.java | 20 +- .../search/highlight/HighlighterTest.java | 16 +- .../classic/TestMultiPhraseQueryParsing.java | 10 +- .../queryparser/classic/TestQueryParser.java | 28 +-- .../lucene/search/TestTermAutomatonQuery.java | 8 +- 12 files changed, 199 insertions(+), 183 deletions(-) diff --git a/lucene/core/src/test/org/apache/lucene/search/TestComplexExplanations.java b/lucene/core/src/test/org/apache/lucene/search/TestComplexExplanations.java index 014566c7c60a..1a4591d5d552 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestComplexExplanations.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestComplexExplanations.java @@ -231,11 +231,11 @@ public void testDMQ10() throws Exception { } public void testMPQ7() throws Exception { - MultiPhraseQuery q = new MultiPhraseQuery(); - q.add(ta(new String[] {"w1"})); - q.add(ta(new String[] {"w2"})); - q.setSlop(1); - bqtest(new BoostQuery(q, 0), new int[] { 0,1,2 }); + MultiPhraseQuery.Builder qb = new MultiPhraseQuery.Builder(); + qb.add(ta(new String[] {"w1"})); + qb.add(ta(new String[] {"w2"})); + qb.setSlop(1); + bqtest(new BoostQuery(qb.build(), 0), new int[] { 0,1,2 }); } public void testBQ12() throws Exception { diff --git a/lucene/core/src/test/org/apache/lucene/search/TestMultiPhraseQuery.java b/lucene/core/src/test/org/apache/lucene/search/TestMultiPhraseQuery.java index 5c81fed6d144..942ac1353e5f 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestMultiPhraseQuery.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestMultiPhraseQuery.java @@ -62,11 +62,11 @@ public void testPhrasePrefix() throws IOException { IndexSearcher searcher = newSearcher(reader); // search for "blueberry pi*": - MultiPhraseQuery query1 = new MultiPhraseQuery(); + MultiPhraseQuery.Builder query1builder = new MultiPhraseQuery.Builder(); // search for "strawberry pi*": - MultiPhraseQuery query2 = new MultiPhraseQuery(); - query1.add(new Term("body", "blueberry")); - query2.add(new Term("body", "strawberry")); + MultiPhraseQuery.Builder query2builder = new MultiPhraseQuery.Builder(); + query1builder.add(new Term("body", "blueberry")); + query2builder.add(new Term("body", "strawberry")); LinkedList termsWithPrefix = new LinkedList<>(); @@ -83,11 +83,13 @@ public void testPhrasePrefix() throws IOException { } } while (te.next() != null); - query1.add(termsWithPrefix.toArray(new Term[0])); + query1builder.add(termsWithPrefix.toArray(new Term[0])); + MultiPhraseQuery query1 = query1builder.build(); assertEquals("body:\"blueberry (piccadilly pie pizza)\"", query1.toString()); - query2.add(termsWithPrefix.toArray(new Term[0])); - assertEquals("body:\"strawberry (piccadilly pie pizza)\"", query2 - .toString()); + + query2builder.add(termsWithPrefix.toArray(new Term[0])); + MultiPhraseQuery query2 = query2builder.build(); + assertEquals("body:\"strawberry (piccadilly pie pizza)\"", query2.toString()); ScoreDoc[] result; result = searcher.search(query1, 1000).scoreDocs; @@ -96,7 +98,7 @@ public void testPhrasePrefix() throws IOException { assertEquals(0, result.length); // search for "blue* pizza": - MultiPhraseQuery query3 = new MultiPhraseQuery(); + MultiPhraseQuery.Builder query3builder = new MultiPhraseQuery.Builder(); termsWithPrefix.clear(); prefix = "blue"; te.seekCeil(new BytesRef(prefix)); @@ -107,15 +109,18 @@ public void testPhrasePrefix() throws IOException { } } while (te.next() != null); - query3.add(termsWithPrefix.toArray(new Term[0])); - query3.add(new Term("body", "pizza")); + query3builder.add(termsWithPrefix.toArray(new Term[0])); + query3builder.add(new Term("body", "pizza")); + + MultiPhraseQuery query3 = query3builder.build(); result = searcher.search(query3, 1000).scoreDocs; assertEquals(2, result.length); // blueberry pizza, bluebird pizza assertEquals("body:\"(blueberry bluebird) pizza\"", query3.toString()); // test slop: - query3.setSlop(1); + query3builder.setSlop(1); + query3 = query3builder.build(); result = searcher.search(query3, 1000).scoreDocs; // just make sure no exc: @@ -124,10 +129,10 @@ public void testPhrasePrefix() throws IOException { assertEquals(3, result.length); // blueberry pizza, bluebird pizza, bluebird // foobar pizza - MultiPhraseQuery query4 = new MultiPhraseQuery(); + MultiPhraseQuery.Builder query4builder = new MultiPhraseQuery.Builder(); expectThrows(IllegalArgumentException.class, () -> { - query4.add(new Term("field1", "foo")); - query4.add(new Term("field2", "foobar")); + query4builder.add(new Term("field1", "foo")); + query4builder.add(new Term("field2", "foobar")); }); writer.close(); @@ -145,11 +150,11 @@ public void testTall() throws IOException { writer.close(); IndexSearcher searcher = newSearcher(r); - MultiPhraseQuery q = new MultiPhraseQuery(); - q.add(new Term("body", "blueberry")); - q.add(new Term("body", "chocolate")); - q.add(new Term[] {new Term("body", "pie"), new Term("body", "tart")}); - assertEquals(2, searcher.search(q, 1).totalHits); + MultiPhraseQuery.Builder qb = new MultiPhraseQuery.Builder(); + qb.add(new Term("body", "blueberry")); + qb.add(new Term("body", "chocolate")); + qb.add(new Term[] {new Term("body", "pie"), new Term("body", "tart")}); + assertEquals(2, searcher.search(qb.build(), 1).totalHits); r.close(); indexStore.close(); } @@ -164,12 +169,12 @@ public void testMultiSloppyWithRepeats() throws IOException { IndexSearcher searcher = newSearcher(r); - MultiPhraseQuery q = new MultiPhraseQuery(); + MultiPhraseQuery.Builder qb = new MultiPhraseQuery.Builder(); // this will fail, when the scorer would propagate [a] rather than [a,b], - q.add(new Term[] {new Term("body", "a"), new Term("body", "b")}); - q.add(new Term[] {new Term("body", "a")}); - q.setSlop(6); - assertEquals(1, searcher.search(q, 1).totalHits); // should match on "a b" + qb.add(new Term[] {new Term("body", "a"), new Term("body", "b")}); + qb.add(new Term[] {new Term("body", "a")}); + qb.setSlop(6); + assertEquals(1, searcher.search(qb.build(), 1).totalHits); // should match on "a b" r.close(); indexStore.close(); @@ -183,10 +188,10 @@ public void testMultiExactWithRepeats() throws IOException { writer.close(); IndexSearcher searcher = newSearcher(r); - MultiPhraseQuery q = new MultiPhraseQuery(); - q.add(new Term[] {new Term("body", "a"), new Term("body", "d")}, 0); - q.add(new Term[] {new Term("body", "a"), new Term("body", "f")}, 2); - assertEquals(1, searcher.search(q, 1).totalHits); // should match on "a b" + MultiPhraseQuery.Builder qb = new MultiPhraseQuery.Builder(); + qb.add(new Term[] {new Term("body", "a"), new Term("body", "d")}, 0); + qb.add(new Term[] {new Term("body", "a"), new Term("body", "f")}, 2); + assertEquals(1, searcher.search(qb.build(), 1).totalHits); // should match on "a b" r.close(); indexStore.close(); } @@ -215,10 +220,10 @@ public void testBooleanQueryContainingSingleTermPrefixQuery() BooleanQuery.Builder q = new BooleanQuery.Builder(); q.add(new TermQuery(new Term("body", "pie")), BooleanClause.Occur.MUST); - MultiPhraseQuery trouble = new MultiPhraseQuery(); - trouble.add(new Term[] {new Term("body", "blueberry"), + MultiPhraseQuery.Builder troubleBuilder = new MultiPhraseQuery.Builder(); + troubleBuilder.add(new Term[] {new Term("body", "blueberry"), new Term("body", "blue")}); - q.add(trouble, BooleanClause.Occur.MUST); + q.add(troubleBuilder.build(), BooleanClause.Occur.MUST); // exception will be thrown here without fix ScoreDoc[] hits = searcher.search(q.build(), 1000).scoreDocs; @@ -246,11 +251,11 @@ public void testPhrasePrefixWithBooleanQuery() throws IOException { BooleanQuery.Builder q = new BooleanQuery.Builder(); q.add(new TermQuery(new Term("type", "note")), BooleanClause.Occur.MUST); - MultiPhraseQuery trouble = new MultiPhraseQuery(); - trouble.add(new Term("body", "a")); - trouble + MultiPhraseQuery.Builder troubleBuilder = new MultiPhraseQuery.Builder(); + troubleBuilder.add(new Term("body", "a")); + troubleBuilder .add(new Term[] {new Term("body", "test"), new Term("body", "this")}); - q.add(trouble, BooleanClause.Occur.MUST); + q.add(troubleBuilder.build(), BooleanClause.Occur.MUST); // exception will be thrown here without fix for #35626: ScoreDoc[] hits = searcher.search(q.build(), 1000).scoreDocs; @@ -268,9 +273,10 @@ public void testNoDocs() throws Exception { IndexReader reader = writer.getReader(); IndexSearcher searcher = newSearcher(reader); - MultiPhraseQuery q = new MultiPhraseQuery(); - q.add(new Term("body", "a")); - q.add(new Term[] {new Term("body", "nope"), new Term("body", "nope")}); + MultiPhraseQuery.Builder qb = new MultiPhraseQuery.Builder(); + qb.add(new Term("body", "a")); + qb.add(new Term[] {new Term("body", "nope"), new Term("body", "nope")}); + MultiPhraseQuery q = qb.build(); assertEquals("Wrong number of hits", 0, searcher.search(q, 1).totalHits); @@ -283,28 +289,36 @@ public void testNoDocs() throws Exception { } public void testHashCodeAndEquals() { - MultiPhraseQuery query1 = new MultiPhraseQuery(); - MultiPhraseQuery query2 = new MultiPhraseQuery(); + MultiPhraseQuery.Builder query1builder = new MultiPhraseQuery.Builder(); + MultiPhraseQuery query1 = query1builder.build(); + + MultiPhraseQuery.Builder query2builder = new MultiPhraseQuery.Builder(); + MultiPhraseQuery query2 = query2builder.build(); assertEquals(query1.hashCode(), query2.hashCode()); assertEquals(query1, query2); Term term1 = new Term("someField", "someText"); - query1.add(term1); - query2.add(term1); + query1builder.add(term1); + query1 = query1builder.build(); + + query2builder.add(term1); + query2 = query2builder.build(); assertEquals(query1.hashCode(), query2.hashCode()); assertEquals(query1, query2); Term term2 = new Term("someField", "someMoreText"); - query1.add(term2); + query1builder.add(term2); + query1 = query1builder.build(); assertFalse(query1.hashCode() == query2.hashCode()); assertFalse(query1.equals(query2)); - query2.add(term2); + query2builder.add(term2); + query2 = query2builder.build(); assertEquals(query1.hashCode(), query2.hashCode()); assertEquals(query1, query2); @@ -320,7 +334,7 @@ private void add(String s, String type, RandomIndexWriter writer) // LUCENE-2526 public void testEmptyToString() { - new MultiPhraseQuery().toString(); + new MultiPhraseQuery.Builder().build().toString(); } public void testCustomIDF() throws Exception { @@ -338,10 +352,10 @@ public Explanation idfExplain(CollectionStatistics collectionStats, TermStatisti } }); - MultiPhraseQuery query = new MultiPhraseQuery(); - query.add(new Term[] { new Term("body", "this"), new Term("body", "that") }); - query.add(new Term("body", "is")); - Weight weight = query.createWeight(searcher, true); + MultiPhraseQuery.Builder queryBuilder = new MultiPhraseQuery.Builder(); + queryBuilder.add(new Term[] { new Term("body", "this"), new Term("body", "that") }); + queryBuilder.add(new Term("body", "is")); + Weight weight = queryBuilder.build().createWeight(searcher, true); assertEquals(10f * 10f, weight.getValueForNormalization(), 0.001f); writer.close(); @@ -372,7 +386,7 @@ public void testZeroPosIncr() throws IOException { IndexReader r = writer.getReader(); writer.close(); IndexSearcher s = newSearcher(r); - MultiPhraseQuery mpq = new MultiPhraseQuery(); + MultiPhraseQuery.Builder mpqb = new MultiPhraseQuery.Builder(); //mpq.setSlop(1); // NOTE: not great that if we do the else clause here we @@ -382,13 +396,13 @@ public void testZeroPosIncr() throws IOException { // return the same position more than once (0, in this // case): if (true) { - mpq.add(new Term[] {new Term("field", "b"), new Term("field", "c")}, 0); - mpq.add(new Term[] {new Term("field", "a")}, 0); + mpqb.add(new Term[] {new Term("field", "b"), new Term("field", "c")}, 0); + mpqb.add(new Term[] {new Term("field", "a")}, 0); } else { - mpq.add(new Term[] {new Term("field", "a")}, 0); - mpq.add(new Term[] {new Term("field", "b"), new Term("field", "c")}, 0); + mpqb.add(new Term[] {new Term("field", "a")}, 0); + mpqb.add(new Term[] {new Term("field", "b"), new Term("field", "c")}, 0); } - TopDocs hits = s.search(mpq, 2); + TopDocs hits = s.search(mpqb.build(), 2); assertEquals(2, hits.totalHits); assertEquals(hits.scoreDocs[0].score, hits.scoreDocs[1].score, 1e-5); /* @@ -449,15 +463,15 @@ private static Token makeToken(String text, int posIncr) { * in each position - one of each position is sufficient (OR logic) */ public void testZeroPosIncrSloppyParsedAnd() throws IOException { - MultiPhraseQuery q = new MultiPhraseQuery(); - q.add(new Term[]{ new Term("field", "a"), new Term("field", "1") }, -1); - q.add(new Term[]{ new Term("field", "b"), new Term("field", "1") }, 0); - q.add(new Term[]{ new Term("field", "c") }, 1); - doTestZeroPosIncrSloppy(q, 0); - q.setSlop(1); - doTestZeroPosIncrSloppy(q, 0); - q.setSlop(2); - doTestZeroPosIncrSloppy(q, 1); + MultiPhraseQuery.Builder qb = new MultiPhraseQuery.Builder(); + qb.add(new Term[]{ new Term("field", "a"), new Term("field", "1") }, -1); + qb.add(new Term[]{ new Term("field", "b"), new Term("field", "1") }, 0); + qb.add(new Term[]{ new Term("field", "c") }, 1); + doTestZeroPosIncrSloppy(qb.build(), 0); + qb.setSlop(1); + doTestZeroPosIncrSloppy(qb.build(), 0); + qb.setSlop(2); + doTestZeroPosIncrSloppy(qb.build(), 1); } private void doTestZeroPosIncrSloppy(Query q, int nExpected) throws IOException { @@ -511,49 +525,49 @@ public void testZeroPosIncrSloppyPqAnd() throws IOException { * MPQ AND Mode - Manually creating a multiple phrase query */ public void testZeroPosIncrSloppyMpqAnd() throws IOException { - final MultiPhraseQuery mpq = new MultiPhraseQuery(); + final MultiPhraseQuery.Builder mpqb = new MultiPhraseQuery.Builder(); int pos = -1; for (Token tap : INCR_0_QUERY_TOKENS_AND) { pos += tap.getPositionIncrement(); - mpq.add(new Term[]{new Term("field",tap.toString())}, pos); //AND logic + mpqb.add(new Term[]{new Term("field",tap.toString())}, pos); //AND logic } - doTestZeroPosIncrSloppy(mpq, 0); - mpq.setSlop(1); - doTestZeroPosIncrSloppy(mpq, 0); - mpq.setSlop(2); - doTestZeroPosIncrSloppy(mpq, 1); + doTestZeroPosIncrSloppy(mpqb.build(), 0); + mpqb.setSlop(1); + doTestZeroPosIncrSloppy(mpqb.build(), 0); + mpqb.setSlop(2); + doTestZeroPosIncrSloppy(mpqb.build(), 1); } /** * MPQ Combined AND OR Mode - Manually creating a multiple phrase query */ public void testZeroPosIncrSloppyMpqAndOrMatch() throws IOException { - final MultiPhraseQuery mpq = new MultiPhraseQuery(); + final MultiPhraseQuery.Builder mpqb = new MultiPhraseQuery.Builder(); for (Token tap[] : INCR_0_QUERY_TOKENS_AND_OR_MATCH) { Term[] terms = tapTerms(tap); final int pos = tap[0].getPositionIncrement()-1; - mpq.add(terms, pos); //AND logic in pos, OR across lines + mpqb.add(terms, pos); //AND logic in pos, OR across lines } - doTestZeroPosIncrSloppy(mpq, 0); - mpq.setSlop(1); - doTestZeroPosIncrSloppy(mpq, 0); - mpq.setSlop(2); - doTestZeroPosIncrSloppy(mpq, 1); + doTestZeroPosIncrSloppy(mpqb.build(), 0); + mpqb.setSlop(1); + doTestZeroPosIncrSloppy(mpqb.build(), 0); + mpqb.setSlop(2); + doTestZeroPosIncrSloppy(mpqb.build(), 1); } /** * MPQ Combined AND OR Mode - Manually creating a multiple phrase query - with no match */ public void testZeroPosIncrSloppyMpqAndOrNoMatch() throws IOException { - final MultiPhraseQuery mpq = new MultiPhraseQuery(); + final MultiPhraseQuery.Builder mpqb = new MultiPhraseQuery.Builder(); for (Token tap[] : INCR_0_QUERY_TOKENS_AND_OR_NO_MATCHN) { Term[] terms = tapTerms(tap); final int pos = tap[0].getPositionIncrement()-1; - mpq.add(terms, pos); //AND logic in pos, OR across lines + mpqb.add(terms, pos); //AND logic in pos, OR across lines } - doTestZeroPosIncrSloppy(mpq, 0); - mpq.setSlop(2); - doTestZeroPosIncrSloppy(mpq, 0); + doTestZeroPosIncrSloppy(mpqb.build(), 0); + mpqb.setSlop(2); + doTestZeroPosIncrSloppy(mpqb.build(), 0); } private Term[] tapTerms(Token[] tap) { @@ -565,11 +579,11 @@ private Term[] tapTerms(Token[] tap) { } public void testNegativeSlop() throws Exception { - MultiPhraseQuery query = new MultiPhraseQuery(); - query.add(new Term("field", "two")); - query.add(new Term("field", "one")); + MultiPhraseQuery.Builder queryBuilder = new MultiPhraseQuery.Builder(); + queryBuilder.add(new Term("field", "two")); + queryBuilder.add(new Term("field", "one")); expectThrows(IllegalArgumentException.class, () -> { - query.setSlop(-2); + queryBuilder.setSlop(-2); }); } diff --git a/lucene/core/src/test/org/apache/lucene/search/TestPhrasePrefixQuery.java b/lucene/core/src/test/org/apache/lucene/search/TestPhrasePrefixQuery.java index 3e8f9e1f84e2..81511509377c 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestPhrasePrefixQuery.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestPhrasePrefixQuery.java @@ -63,11 +63,11 @@ public void testPhrasePrefix() throws IOException { IndexSearcher searcher = newSearcher(reader); // PhrasePrefixQuery query1 = new PhrasePrefixQuery(); - MultiPhraseQuery query1 = new MultiPhraseQuery(); + MultiPhraseQuery.Builder query1builder = new MultiPhraseQuery.Builder(); // PhrasePrefixQuery query2 = new PhrasePrefixQuery(); - MultiPhraseQuery query2 = new MultiPhraseQuery(); - query1.add(new Term("body", "blueberry")); - query2.add(new Term("body", "strawberry")); + MultiPhraseQuery.Builder query2builder = new MultiPhraseQuery.Builder(); + query1builder.add(new Term("body", "blueberry")); + query2builder.add(new Term("body", "strawberry")); LinkedList termsWithPrefix = new LinkedList<>(); @@ -84,14 +84,14 @@ public void testPhrasePrefix() throws IOException { } } while (te.next() != null); - query1.add(termsWithPrefix.toArray(new Term[0])); - query2.add(termsWithPrefix.toArray(new Term[0])); + query1builder.add(termsWithPrefix.toArray(new Term[0])); + query2builder.add(termsWithPrefix.toArray(new Term[0])); ScoreDoc[] result; - result = searcher.search(query1, 1000).scoreDocs; + result = searcher.search(query1builder.build(), 1000).scoreDocs; assertEquals(2, result.length); - result = searcher.search(query2, 1000).scoreDocs; + result = searcher.search(query2builder.build(), 1000).scoreDocs; assertEquals(0, result.length); reader.close(); indexStore.close(); diff --git a/lucene/core/src/test/org/apache/lucene/search/TestPositionIncrement.java b/lucene/core/src/test/org/apache/lucene/search/TestPositionIncrement.java index 84468da4d3bd..b7ae42a54bca 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestPositionIncrement.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestPositionIncrement.java @@ -175,9 +175,9 @@ public void reset() throws IOException { // multi-phrase query should succed for non existing searched term // because there exist another searched terms in the same searched position. - MultiPhraseQuery mq = new MultiPhraseQuery(); - mq.add(new Term[]{new Term("field", "3"),new Term("field", "9")},0); - hits = searcher.search(mq, 1000).scoreDocs; + MultiPhraseQuery.Builder mqb = new MultiPhraseQuery.Builder(); + mqb.add(new Term[]{new Term("field", "3"),new Term("field", "9")},0); + hits = searcher.search(mqb.build(), 1000).scoreDocs; assertEquals(1, hits.length); q = new PhraseQuery("field", "2", "4"); diff --git a/lucene/core/src/test/org/apache/lucene/search/TestSimpleExplanations.java b/lucene/core/src/test/org/apache/lucene/search/TestSimpleExplanations.java index 9274fdfef8a0..a38884238736 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestSimpleExplanations.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestSimpleExplanations.java @@ -200,42 +200,42 @@ public void testDMQ9() throws Exception { /* MultiPhraseQuery */ public void testMPQ1() throws Exception { - MultiPhraseQuery q = new MultiPhraseQuery(); - q.add(ta(new String[] {"w1"})); - q.add(ta(new String[] {"w2","w3", "xx"})); - qtest(q, new int[] { 0,1,2,3 }); + MultiPhraseQuery.Builder qb = new MultiPhraseQuery.Builder(); + qb.add(ta(new String[] {"w1"})); + qb.add(ta(new String[] {"w2","w3", "xx"})); + qtest(qb.build(), new int[] { 0,1,2,3 }); } public void testMPQ2() throws Exception { - MultiPhraseQuery q = new MultiPhraseQuery(); - q.add(ta(new String[] {"w1"})); - q.add(ta(new String[] {"w2","w3"})); - qtest(q, new int[] { 0,1,3 }); + MultiPhraseQuery.Builder qb = new MultiPhraseQuery.Builder(); + qb.add(ta(new String[] {"w1"})); + qb.add(ta(new String[] {"w2","w3"})); + qtest(qb.build(), new int[] { 0,1,3 }); } public void testMPQ3() throws Exception { - MultiPhraseQuery q = new MultiPhraseQuery(); - q.add(ta(new String[] {"w1","xx"})); - q.add(ta(new String[] {"w2","w3"})); - qtest(q, new int[] { 0,1,2,3 }); + MultiPhraseQuery.Builder qb = new MultiPhraseQuery.Builder(); + qb.add(ta(new String[] {"w1","xx"})); + qb.add(ta(new String[] {"w2","w3"})); + qtest(qb.build(), new int[] { 0,1,2,3 }); } public void testMPQ4() throws Exception { - MultiPhraseQuery q = new MultiPhraseQuery(); - q.add(ta(new String[] {"w1"})); - q.add(ta(new String[] {"w2"})); - qtest(q, new int[] { 0 }); + MultiPhraseQuery.Builder qb = new MultiPhraseQuery.Builder(); + qb.add(ta(new String[] {"w1"})); + qb.add(ta(new String[] {"w2"})); + qtest(qb.build(), new int[] { 0 }); } public void testMPQ5() throws Exception { - MultiPhraseQuery q = new MultiPhraseQuery(); - q.add(ta(new String[] {"w1"})); - q.add(ta(new String[] {"w2"})); - q.setSlop(1); - qtest(q, new int[] { 0,1,2 }); + MultiPhraseQuery.Builder qb = new MultiPhraseQuery.Builder(); + qb.add(ta(new String[] {"w1"})); + qb.add(ta(new String[] {"w2"})); + qb.setSlop(1); + qtest(qb.build(), new int[] { 0,1,2 }); } public void testMPQ6() throws Exception { - MultiPhraseQuery q = new MultiPhraseQuery(); - q.add(ta(new String[] {"w1","w3"})); - q.add(ta(new String[] {"w2"})); - q.setSlop(1); - qtest(q, new int[] { 0,1,2,3 }); + MultiPhraseQuery.Builder qb = new MultiPhraseQuery.Builder(); + qb.add(ta(new String[] {"w1","w3"})); + qb.add(ta(new String[] {"w2"})); + qb.setSlop(1); + qtest(qb.build(), new int[] { 0,1,2,3 }); } /* some simple tests of boolean queries containing term queries */ diff --git a/lucene/core/src/test/org/apache/lucene/search/TestSimpleSearchEquivalence.java b/lucene/core/src/test/org/apache/lucene/search/TestSimpleSearchEquivalence.java index 33007eb64086..6386c79967c2 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestSimpleSearchEquivalence.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestSimpleSearchEquivalence.java @@ -145,10 +145,10 @@ public void testExactPhraseVersusMultiPhrase() throws Exception { Term t2 = randomTerm(); PhraseQuery q1 = new PhraseQuery(t1.field(), t1.bytes(), t2.bytes()); Term t3 = randomTerm(); - MultiPhraseQuery q2 = new MultiPhraseQuery(); - q2.add(t1); - q2.add(new Term[] { t2, t3 }); - assertSubsetOf(q1, q2); + MultiPhraseQuery.Builder q2b = new MultiPhraseQuery.Builder(); + q2b.add(t1); + q2b.add(new Term[] { t2, t3 }); + assertSubsetOf(q1, q2b.build()); } /** same as above, with posincs */ @@ -160,10 +160,10 @@ public void testExactPhraseVersusMultiPhraseWithHoles() throws Exception { builder.add(t2, 2); PhraseQuery q1 = builder.build(); Term t3 = randomTerm(); - MultiPhraseQuery q2 = new MultiPhraseQuery(); - q2.add(t1); - q2.add(new Term[] { t2, t3 }, 2); - assertSubsetOf(q1, q2); + MultiPhraseQuery.Builder q2b = new MultiPhraseQuery.Builder(); + q2b.add(t1); + q2b.add(new Term[] { t2, t3 }, 2); + assertSubsetOf(q1, q2b.build()); } /** "A B"~∞ = +A +B if A != B */ diff --git a/lucene/core/src/test/org/apache/lucene/search/TestSloppyPhraseQuery2.java b/lucene/core/src/test/org/apache/lucene/search/TestSloppyPhraseQuery2.java index 3910fac45189..1a97a99e36a5 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestSloppyPhraseQuery2.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestSloppyPhraseQuery2.java @@ -147,8 +147,8 @@ public void testRandomIncreasingSloppiness() throws Exception { for (int i = 0; i < 10; i++) { MultiPhraseQuery q1 = randomPhraseQuery(seed); MultiPhraseQuery q2 = randomPhraseQuery(seed); - q1.setSlop(i); - q2.setSlop(i+1); + q1 = new MultiPhraseQuery.Builder(q1).setSlop(i).build(); + q2 = new MultiPhraseQuery.Builder(q2).setSlop(i+1).build(); assertSubsetOf(q1, q2); } } @@ -156,7 +156,7 @@ public void testRandomIncreasingSloppiness() throws Exception { private MultiPhraseQuery randomPhraseQuery(long seed) { Random random = new Random(seed); int length = TestUtil.nextInt(random, 2, 5); - MultiPhraseQuery pq = new MultiPhraseQuery(); + MultiPhraseQuery.Builder pqb = new MultiPhraseQuery.Builder(); int position = 0; for (int i = 0; i < length; i++) { int depth = TestUtil.nextInt(random, 1, 3); @@ -164,9 +164,9 @@ private MultiPhraseQuery randomPhraseQuery(long seed) { for (int j = 0; j < depth; j++) { terms[j] = new Term("field", "" + (char) TestUtil.nextInt(random, 'a', 'z')); } - pq.add(terms, position); + pqb.add(terms, position); position += TestUtil.nextInt(random, 1, 3); } - return pq; + return pqb.build(); } } diff --git a/lucene/core/src/test/org/apache/lucene/util/TestQueryBuilder.java b/lucene/core/src/test/org/apache/lucene/util/TestQueryBuilder.java index bd5a49a0a12f..205fbab09814 100644 --- a/lucene/core/src/test/org/apache/lucene/util/TestQueryBuilder.java +++ b/lucene/core/src/test/org/apache/lucene/util/TestQueryBuilder.java @@ -173,11 +173,11 @@ public void testSynonyms() throws Exception { /** forms multiphrase query */ public void testSynonymsPhrase() throws Exception { - MultiPhraseQuery expected = new MultiPhraseQuery(); - expected.add(new Term("field", "old")); - expected.add(new Term[] { new Term("field", "dogs"), new Term("field", "dog") }); + MultiPhraseQuery.Builder expectedBuilder = new MultiPhraseQuery.Builder(); + expectedBuilder.add(new Term("field", "old")); + expectedBuilder.add(new Term[] { new Term("field", "dogs"), new Term("field", "dog") }); QueryBuilder builder = new QueryBuilder(new MockSynonymAnalyzer()); - assertEquals(expected, builder.createPhraseQuery("field", "old dogs")); + assertEquals(expectedBuilder.build(), builder.createPhraseQuery("field", "old dogs")); } protected static class SimpleCJKTokenizer extends Tokenizer { @@ -331,13 +331,13 @@ public void testCJKSynonymsAND2() throws Exception { /** forms multiphrase query */ public void testCJKSynonymsPhrase() throws Exception { - MultiPhraseQuery expected = new MultiPhraseQuery(); - expected.add(new Term("field", "中")); - expected.add(new Term[] { new Term("field", "国"), new Term("field", "國")}); + MultiPhraseQuery.Builder expectedBuilder = new MultiPhraseQuery.Builder(); + expectedBuilder.add(new Term("field", "中")); + expectedBuilder.add(new Term[] { new Term("field", "国"), new Term("field", "國")}); QueryBuilder builder = new QueryBuilder(new MockCJKSynonymAnalyzer()); - assertEquals(expected, builder.createPhraseQuery("field", "中国")); - expected.setSlop(3); - assertEquals(expected, builder.createPhraseQuery("field", "中国", 3)); + assertEquals(expectedBuilder.build(), builder.createPhraseQuery("field", "中国")); + expectedBuilder.setSlop(3); + assertEquals(expectedBuilder.build(), builder.createPhraseQuery("field", "中国", 3)); } public void testNoTermAttribute() { diff --git a/lucene/highlighter/src/test/org/apache/lucene/search/highlight/HighlighterTest.java b/lucene/highlighter/src/test/org/apache/lucene/search/highlight/HighlighterTest.java index edc91f6e9d3e..632729bcb237 100644 --- a/lucene/highlighter/src/test/org/apache/lucene/search/highlight/HighlighterTest.java +++ b/lucene/highlighter/src/test/org/apache/lucene/search/highlight/HighlighterTest.java @@ -793,29 +793,29 @@ public void testPosTermStdTerm() throws Exception { } public void testQueryScorerMultiPhraseQueryHighlighting() throws Exception { - MultiPhraseQuery mpq = new MultiPhraseQuery(); + MultiPhraseQuery.Builder mpqb = new MultiPhraseQuery.Builder(); - mpq.add(new Term[] { new Term(FIELD_NAME, "wordx"), new Term(FIELD_NAME, "wordb") }); - mpq.add(new Term(FIELD_NAME, "wordy")); + mpqb.add(new Term[] { new Term(FIELD_NAME, "wordx"), new Term(FIELD_NAME, "wordb") }); + mpqb.add(new Term(FIELD_NAME, "wordy")); - doSearching(mpq); + doSearching(mpqb.build()); final int maxNumFragmentsRequired = 2; assertExpectedHighlightCount(maxNumFragmentsRequired, 6); } public void testQueryScorerMultiPhraseQueryHighlightingWithGap() throws Exception { - MultiPhraseQuery mpq = new MultiPhraseQuery(); + MultiPhraseQuery.Builder mpqb = new MultiPhraseQuery.Builder(); /* * The toString of MultiPhraseQuery doesn't work so well with these * out-of-order additions, but the Query itself seems to match accurately. */ - mpq.add(new Term[] { new Term(FIELD_NAME, "wordz") }, 2); - mpq.add(new Term[] { new Term(FIELD_NAME, "wordx") }, 0); + mpqb.add(new Term[] { new Term(FIELD_NAME, "wordz") }, 2); + mpqb.add(new Term[] { new Term(FIELD_NAME, "wordx") }, 0); - doSearching(mpq); + doSearching(mpqb.build()); final int maxNumFragmentsRequired = 1; final int expectedHighlights = 2; diff --git a/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestMultiPhraseQueryParsing.java b/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestMultiPhraseQueryParsing.java index 709f3c387393..16510380643b 100644 --- a/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestMultiPhraseQueryParsing.java +++ b/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestMultiPhraseQueryParsing.java @@ -100,12 +100,12 @@ public void testMultiPhraseQueryParsing() throws Exception { Query q = qp.parse("\"this text is acually ignored\""); assertTrue("wrong query type!", q instanceof MultiPhraseQuery); - MultiPhraseQuery multiPhraseQuery = new MultiPhraseQuery(); - multiPhraseQuery.add(new Term[]{ new Term("field", "a"), new Term("field", "1") }, -1); - multiPhraseQuery.add(new Term[]{ new Term("field", "b"), new Term("field", "1") }, 0); - multiPhraseQuery.add(new Term[]{ new Term("field", "c") }, 1); + MultiPhraseQuery.Builder multiPhraseQueryBuilder = new MultiPhraseQuery.Builder(); + multiPhraseQueryBuilder.add(new Term[]{ new Term("field", "a"), new Term("field", "1") }, -1); + multiPhraseQueryBuilder.add(new Term[]{ new Term("field", "b"), new Term("field", "1") }, 0); + multiPhraseQueryBuilder.add(new Term[]{ new Term("field", "c") }, 1); - assertEquals(multiPhraseQuery, q); + assertEquals(multiPhraseQueryBuilder.build(), q); } } diff --git a/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestQueryParser.java b/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestQueryParser.java index 2457d3ca81f0..5b4eba87994e 100644 --- a/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestQueryParser.java +++ b/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestQueryParser.java @@ -338,16 +338,17 @@ public void testSynonyms() throws Exception { /** forms multiphrase query */ public void testSynonymsPhrase() throws Exception { - MultiPhraseQuery expectedQ = new MultiPhraseQuery(); - expectedQ.add(new Term("field", "old")); - expectedQ.add(new Term[] { new Term("field", "dogs"), new Term("field", "dog") }); + MultiPhraseQuery.Builder expectedQBuilder = new MultiPhraseQuery.Builder(); + expectedQBuilder.add(new Term("field", "old")); + expectedQBuilder.add(new Term[] { new Term("field", "dogs"), new Term("field", "dog") }); QueryParser qp = new QueryParser("field", new MockSynonymAnalyzer()); - assertEquals(expectedQ, qp.parse("\"old dogs\"")); + assertEquals(expectedQBuilder.build(), qp.parse("\"old dogs\"")); qp.setDefaultOperator(Operator.AND); - assertEquals(expectedQ, qp.parse("\"old dogs\"")); - BoostQuery expected = new BoostQuery(expectedQ, 2f); + assertEquals(expectedQBuilder.build(), qp.parse("\"old dogs\"")); + BoostQuery expected = new BoostQuery(expectedQBuilder.build(), 2f); assertEquals(expected, qp.parse("\"old dogs\"^2")); - expectedQ.setSlop(3); + expectedQBuilder.setSlop(3); + expected = new BoostQuery(expectedQBuilder.build(), 2f); assertEquals(expected, qp.parse("\"old dogs\"~3^2")); } @@ -461,15 +462,16 @@ public void testCJKSynonymsAND2() throws Exception { /** forms multiphrase query */ public void testCJKSynonymsPhrase() throws Exception { - MultiPhraseQuery expectedQ = new MultiPhraseQuery(); - expectedQ.add(new Term("field", "中")); - expectedQ.add(new Term[] { new Term("field", "国"), new Term("field", "國")}); + MultiPhraseQuery.Builder expectedQBuilder = new MultiPhraseQuery.Builder(); + expectedQBuilder.add(new Term("field", "中")); + expectedQBuilder.add(new Term[] { new Term("field", "国"), new Term("field", "國")}); QueryParser qp = new QueryParser("field", new MockCJKSynonymAnalyzer()); qp.setDefaultOperator(Operator.AND); - assertEquals(expectedQ, qp.parse("\"中国\"")); - Query expected = new BoostQuery(expectedQ, 2f); + assertEquals(expectedQBuilder.build(), qp.parse("\"中国\"")); + Query expected = new BoostQuery(expectedQBuilder.build(), 2f); assertEquals(expected, qp.parse("\"中国\"^2")); - expectedQ.setSlop(3); + expectedQBuilder.setSlop(3); + expected = new BoostQuery(expectedQBuilder.build(), 2f); assertEquals(expected, qp.parse("\"中国\"~3^2")); } diff --git a/lucene/sandbox/src/test/org/apache/lucene/search/TestTermAutomatonQuery.java b/lucene/sandbox/src/test/org/apache/lucene/search/TestTermAutomatonQuery.java index 392941d513d0..8016b6168edf 100644 --- a/lucene/sandbox/src/test/org/apache/lucene/search/TestTermAutomatonQuery.java +++ b/lucene/sandbox/src/test/org/apache/lucene/search/TestTermAutomatonQuery.java @@ -522,15 +522,15 @@ public TokenStreamComponents createComponents(String fieldName) { } } String string = sb.toString(); - MultiPhraseQuery mpq = new MultiPhraseQuery(); + MultiPhraseQuery.Builder mpqb = new MultiPhraseQuery.Builder(); for(int j=0;j Date: Thu, 3 Mar 2016 16:16:41 +0100 Subject: [PATCH 3/3] LUCENE-7064: Modifying Builder no longer affects already created queries --- .../lucene/search/MultiPhraseQuery.java | 100 ++++++++++-------- .../highlight/WeightedSpanTermExtractor.java | 6 +- .../lucene/payloads/PayloadSpanUtil.java | 6 +- .../solr/search/ExtendedDismaxQParser.java | 2 +- 4 files changed, 62 insertions(+), 52 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/search/MultiPhraseQuery.java b/lucene/core/src/java/org/apache/lucene/search/MultiPhraseQuery.java index e571f00ad216..141084f7b3b1 100644 --- a/lucene/core/src/java/org/apache/lucene/search/MultiPhraseQuery.java +++ b/lucene/core/src/java/org/apache/lucene/search/MultiPhraseQuery.java @@ -63,8 +63,17 @@ public Builder() { public Builder(MultiPhraseQuery multiPhraseQuery) { this.field = multiPhraseQuery.field; - this.termArrays = new ArrayList<>(multiPhraseQuery.termArrays); - this.positions = new ArrayList<>(multiPhraseQuery.positions); + + int length = multiPhraseQuery.termArrays.length; + + this.termArrays = new ArrayList<>(length); + this.positions = new ArrayList<>(length); + + for (int i = 0 ; i < length ; ++i) { + this.termArrays.add(multiPhraseQuery.termArrays[i]); + this.positions.add(multiPhraseQuery.positions[i]); + } + this.slop = multiPhraseQuery.slop; } @@ -121,16 +130,24 @@ public Builder add(Term[] terms, int position) { } public MultiPhraseQuery build() { - return new MultiPhraseQuery(field, termArrays, positions, slop); + int[] positionsArray = new int[this.positions.size()]; + + for (int i = 0; i < this.positions.size(); ++i) { + positionsArray[i] = this.positions.get(i); + } + + Term[][] termArraysArray = termArrays.toArray(new Term[termArrays.size()][]); + + return new MultiPhraseQuery(field, termArraysArray, positionsArray, slop); } } private final String field; - private final ArrayList termArrays; // TODO: Change to Term[][] - private final ArrayList positions; // TODO: Change to int[] + private final Term[][] termArrays; + private final int[] positions; private final int slop; - private MultiPhraseQuery(String field, ArrayList termArrays, ArrayList positions, int slop) { + private MultiPhraseQuery(String field, Term[][] termArrays, int[] positions, int slop) { // No argument checks here since they are provided by the MultiPhraseQuery.Builder this.field = field; this.termArrays = termArrays; @@ -144,21 +161,19 @@ private MultiPhraseQuery(String field, ArrayList termArrays, ArrayList getTermArrays() { - return Collections.unmodifiableList(termArrays); + public Term[][] getTermArrays() { + return termArrays; } /** * Returns the relative positions of terms in this phrase. + * Do not modify! */ public int[] getPositions() { - int[] result = new int[positions.size()]; - for (int i = 0; i < positions.size(); i++) - result[i] = positions.get(i); - return result; + return positions; } @@ -211,10 +226,10 @@ public void normalize(float queryNorm, float boost) { @Override public Scorer scorer(LeafReaderContext context) throws IOException { - assert !termArrays.isEmpty(); + assert termArrays.length != 0; final LeafReader reader = context.reader(); - PhraseQuery.PostingsAndFreq[] postingsFreqs = new PhraseQuery.PostingsAndFreq[termArrays.size()]; + PhraseQuery.PostingsAndFreq[] postingsFreqs = new PhraseQuery.PostingsAndFreq[termArrays.length]; final Terms fieldTerms = reader.terms(field); if (fieldTerms == null) { @@ -232,7 +247,7 @@ public Scorer scorer(LeafReaderContext context) throws IOException { float totalMatchCost = 0; for (int pos=0; pos postings = new ArrayList<>(); for (Term term : terms) { @@ -255,7 +270,7 @@ public Scorer scorer(LeafReaderContext context) throws IOException { postingsEnum = new UnionPostingsEnum(postings); } - postingsFreqs[pos] = new PhraseQuery.PostingsAndFreq(postingsEnum, positions.get(pos).intValue(), terms); + postingsFreqs[pos] = new PhraseQuery.PostingsAndFreq(postingsEnum, positions[pos], terms); } // sort by increasing docFreq order @@ -297,10 +312,10 @@ public Explanation explain(LeafReaderContext context, int doc) throws IOExceptio @Override public Query rewrite(IndexReader reader) throws IOException { - if (termArrays.isEmpty()) { + if (termArrays.length == 0) { return new MatchNoDocsQuery(); - } else if (termArrays.size() == 1) { // optimize one-term case - Term[] terms = termArrays.get(0); + } else if (termArrays.length == 1) { // optimize one-term case + Term[] terms = termArrays[0]; BooleanQuery.Builder builder = new BooleanQuery.Builder(); builder.setDisableCoord(true); for (Term term : terms) { @@ -327,16 +342,12 @@ public final String toString(String f) { } buffer.append("\""); - int k = 0; - Iterator i = termArrays.iterator(); int lastPos = -1; - boolean first = true; - while (i.hasNext()) { - Term[] terms = i.next(); - int position = positions.get(k); - if (first) { - first = false; - } else { + + for (int i = 0 ; i < termArrays.length ; ++i) { + Term[] terms = termArrays[i]; + int position = positions[i]; + if (i != 0) { buffer.append(" "); for (int j=1; j<(position-lastPos); j++) { buffer.append("? "); @@ -354,7 +365,6 @@ public final String toString(String f) { buffer.append(terms[0].text()); } lastPos = position; - ++k; } buffer.append("\""); @@ -370,12 +380,13 @@ public final String toString(String f) { /** Returns true if o is equal to this. */ @Override public boolean equals(Object o) { - if (!(o instanceof MultiPhraseQuery)) return false; + if (super.equals(o) == false) { + return false; + } MultiPhraseQuery other = (MultiPhraseQuery)o; - return super.equals(o) - && this.slop == other.slop - && termArraysEquals(this.termArrays, other.termArrays) - && this.positions.equals(other.positions); + return this.slop == other.slop + && termArraysEquals(this.termArrays, other.termArrays) // terms equal implies field equal + && Arrays.equals(this.positions, other.positions); } /** Returns a hash code value for this object.*/ @@ -383,8 +394,8 @@ && termArraysEquals(this.termArrays, other.termArrays) public int hashCode() { return super.hashCode() ^ slop - ^ termArraysHashCode() - ^ positions.hashCode(); + ^ termArraysHashCode() // terms equal implies field equal + ^ Arrays.hashCode(positions); } // Breakout calculation of the termArrays hashcode @@ -398,15 +409,14 @@ private int termArraysHashCode() { } // Breakout calculation of the termArrays equals - private boolean termArraysEquals(List termArrays1, List termArrays2) { - if (termArrays1.size() != termArrays2.size()) { + private boolean termArraysEquals(Term[][] termArrays1, Term[][] termArrays2) { + if (termArrays1.length != termArrays2.length) { return false; } - ListIterator iterator1 = termArrays1.listIterator(); - ListIterator iterator2 = termArrays2.listIterator(); - while (iterator1.hasNext()) { - Term[] termArray1 = iterator1.next(); - Term[] termArray2 = iterator2.next(); + + for (int i = 0 ; i < termArrays1.length ; ++i) { + Term[] termArray1 = termArrays1[i]; + Term[] termArray2 = termArrays2[i]; if (!(termArray1 == null ? termArray2 == null : Arrays.equals(termArray1, termArray2))) { return false; diff --git a/lucene/highlighter/src/java/org/apache/lucene/search/highlight/WeightedSpanTermExtractor.java b/lucene/highlighter/src/java/org/apache/lucene/search/highlight/WeightedSpanTermExtractor.java index a73ebf8ec27f..650be875d88b 100644 --- a/lucene/highlighter/src/java/org/apache/lucene/search/highlight/WeightedSpanTermExtractor.java +++ b/lucene/highlighter/src/java/org/apache/lucene/search/highlight/WeightedSpanTermExtractor.java @@ -156,7 +156,7 @@ protected void extract(Query query, float boost, Map te extract(((ToChildBlockJoinQuery) query).getParentQuery(), boost, terms); } else if (query instanceof MultiPhraseQuery) { final MultiPhraseQuery mpq = (MultiPhraseQuery) query; - final List termArrays = mpq.getTermArrays(); + final Term[][] termArrays = mpq.getTermArrays(); final int[] positions = mpq.getPositions(); if (positions.length > 0) { @@ -171,8 +171,8 @@ protected void extract(Query query, float boost, Map te final List[] disjunctLists = new List[maxPosition + 1]; int distinctPositions = 0; - for (int i = 0; i < termArrays.size(); ++i) { - final Term[] termArray = termArrays.get(i); + for (int i = 0; i < termArrays.length; ++i) { + final Term[] termArray = termArrays[i]; List disjuncts = disjunctLists[positions[i]]; if (disjuncts == null) { disjuncts = (disjunctLists[positions[i]] = new ArrayList<>(termArray.length)); diff --git a/lucene/sandbox/src/java/org/apache/lucene/payloads/PayloadSpanUtil.java b/lucene/sandbox/src/java/org/apache/lucene/payloads/PayloadSpanUtil.java index 400e42cf1f92..20cd2c014282 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/payloads/PayloadSpanUtil.java +++ b/lucene/sandbox/src/java/org/apache/lucene/payloads/PayloadSpanUtil.java @@ -114,7 +114,7 @@ private void queryToSpanQuery(Query query, Collection payloads) } else if (query instanceof MultiPhraseQuery) { final MultiPhraseQuery mpq = (MultiPhraseQuery) query; - final List termArrays = mpq.getTermArrays(); + final Term[][] termArrays = mpq.getTermArrays(); final int[] positions = mpq.getPositions(); if (positions.length > 0) { @@ -129,8 +129,8 @@ private void queryToSpanQuery(Query query, Collection payloads) new List[maxPosition + 1]; int distinctPositions = 0; - for (int i = 0; i < termArrays.size(); ++i) { - final Term[] termArray = termArrays.get(i); + for (int i = 0; i < termArrays.length; ++i) { + final Term[] termArray = termArrays[i]; List disjuncts = disjunctLists[positions[i]]; if (disjuncts == null) { disjuncts = (disjunctLists[positions[i]] = new ArrayList<>( diff --git a/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java b/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java index 71dac7255317..8401f3ece1eb 100644 --- a/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java +++ b/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java @@ -1236,7 +1236,7 @@ private Query getQuery() { query = builder.build(); } else if (query instanceof MultiPhraseQuery) { MultiPhraseQuery mpq = (MultiPhraseQuery)query; - if (minClauseSize > 1 && mpq.getTermArrays().size() < minClauseSize) return null; + if (minClauseSize > 1 && mpq.getTermArrays().length < minClauseSize) return null; if (slop != mpq.getSlop()) { query = new MultiPhraseQuery.Builder(mpq).setSlop(slop).build(); }