diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 8f212ffed54..7fa4ece80d4 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -95,6 +95,8 @@ API Changes AssertingScorer checks that getChildren() is not called on an unpositioned Scorer. (Alan Woodward, Adrien Grand) +* LUCENE-7702: Removed GraphQuery in favour of simple boolean query. (Matt Webber via Jim Ferenczi) + New Features * LUCENE-7449: Add CROSSES relation support to RangeFieldQuery. (Nick Knize) diff --git a/lucene/core/src/java/org/apache/lucene/search/GraphQuery.java b/lucene/core/src/java/org/apache/lucene/search/GraphQuery.java deleted file mode 100644 index a1308c9cb4c..00000000000 --- a/lucene/core/src/java/org/apache/lucene/search/GraphQuery.java +++ /dev/null @@ -1,136 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.lucene.search; - -import java.io.IOException; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import java.util.Objects; - -import org.apache.lucene.index.IndexReader; - -/** - * A query that wraps multiple sub-queries generated from a graph token stream. - */ -public final class GraphQuery extends Query { - private final Query[] queries; - private boolean hasBoolean = false; - private boolean hasPhrase = false; - - /** - * Constructor sets the queries and checks if any of them are - * a boolean query. - * - * @param queries the non-null array of queries - */ - public GraphQuery(Query... queries) { - this.queries = Objects.requireNonNull(queries).clone(); - for (Query query : queries) { - if (query instanceof BooleanQuery) { - hasBoolean = true; - } else if (query instanceof PhraseQuery) { - hasPhrase = true; - } - } - } - - /** - * Gets the queries - * - * @return unmodifiable list of Query - */ - public List getQueries() { - return Collections.unmodifiableList(Arrays.asList(queries)); - } - - /** - * If there is at least one boolean query or not. - * - * @return true if there is a boolean, false if not - */ - public boolean hasBoolean() { - return hasBoolean; - } - - /** - * If there is at least one phrase query or not. - * - * @return true if there is a phrase query, false if not - */ - public boolean hasPhrase() { - return hasPhrase; - } - - /** - * Rewrites to a single query or a boolean query where each query is a SHOULD clause. - */ - @Override - public Query rewrite(IndexReader reader) throws IOException { - if (queries.length == 0) { - return new BooleanQuery.Builder().build(); - } - - if (queries.length == 1) { - return queries[0]; - } - - BooleanQuery.Builder q = new BooleanQuery.Builder(); - for (Query clause : queries) { - q.add(clause, BooleanClause.Occur.SHOULD); - } - - return q.build(); - } - - @Override - public String toString(String field) { - StringBuilder builder = new StringBuilder("Graph("); - for (int i = 0; i < queries.length; i++) { - if (i != 0) { - builder.append(", "); - } - builder.append(Objects.toString(queries[i])); - } - - if (queries.length > 0) { - builder.append(", "); - } - - builder.append("hasBoolean=") - .append(hasBoolean) - .append(", hasPhrase=") - .append(hasPhrase) - .append(")"); - - return builder.toString(); - } - - @Override - public boolean equals(Object other) { - return sameClassAs(other) && - hasBoolean == ((GraphQuery) other).hasBoolean && - hasPhrase == ((GraphQuery) other).hasPhrase && - Arrays.equals(queries, ((GraphQuery) other).queries); - } - - @Override - public int hashCode() { - return 31 * classHash() + Arrays.deepHashCode(new Object[]{hasBoolean, hasPhrase, queries}); - } -} \ No newline at end of file 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 3a8d31c2816..0832bdb2637 100644 --- a/lucene/core/src/java/org/apache/lucene/util/QueryBuilder.java +++ b/lucene/core/src/java/org/apache/lucene/util/QueryBuilder.java @@ -19,6 +19,7 @@ package org.apache.lucene.util; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Iterator; import java.util.List; @@ -31,7 +32,6 @@ import org.apache.lucene.analysis.tokenattributes.TermToBytesRefAttribute; import org.apache.lucene.index.Term; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.GraphQuery; import org.apache.lucene.search.MultiPhraseQuery; import org.apache.lucene.search.PhraseQuery; import org.apache.lucene.search.Query; @@ -637,7 +637,15 @@ public class QueryBuilder { * @return new Query instance */ protected Query newGraphSynonymQuery(Query queries[]) { - return new GraphQuery(queries); + if (queries == null) { + return new BooleanQuery.Builder().build(); + } else if (queries.length == 1) { + return queries[0]; + } else { + BooleanQuery.Builder builder = new BooleanQuery.Builder(); + Arrays.stream(queries).forEachOrdered(qry -> builder.add(qry, BooleanClause.Occur.SHOULD)); + return builder.build(); + } } /** diff --git a/lucene/core/src/test/org/apache/lucene/search/TestGraphQuery.java b/lucene/core/src/test/org/apache/lucene/search/TestGraphQuery.java deleted file mode 100644 index 412fac4654c..00000000000 --- a/lucene/core/src/test/org/apache/lucene/search/TestGraphQuery.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.lucene.search; - - -import java.io.IOException; - -import org.apache.lucene.index.Term; -import org.apache.lucene.util.LuceneTestCase; - -public class TestGraphQuery extends LuceneTestCase { - - public void testEquals() { - QueryUtils.checkEqual(new GraphQuery(), new GraphQuery()); - QueryUtils.checkEqual(new GraphQuery(new MatchAllDocsQuery()), new GraphQuery(new MatchAllDocsQuery())); - QueryUtils.checkEqual( - new GraphQuery(new TermQuery(new Term("a", "a")), new TermQuery(new Term("a", "b"))), - new GraphQuery(new TermQuery(new Term("a", "a")), new TermQuery(new Term("a", "b"))) - ); - } - - public void testBooleanDetection() { - assertFalse(new GraphQuery().hasBoolean()); - assertFalse(new GraphQuery(new MatchAllDocsQuery(), new TermQuery(new Term("a", "a"))).hasBoolean()); - assertTrue(new GraphQuery(new BooleanQuery.Builder().build()).hasBoolean()); - assertTrue(new GraphQuery(new TermQuery(new Term("a", "a")), new BooleanQuery.Builder().build()).hasBoolean()); - } - - public void testPhraseDetection() { - assertFalse(new GraphQuery().hasPhrase()); - assertFalse(new GraphQuery(new MatchAllDocsQuery(), new TermQuery(new Term("a", "a"))).hasPhrase()); - assertTrue(new GraphQuery(new PhraseQuery.Builder().build()).hasPhrase()); - assertTrue(new GraphQuery(new TermQuery(new Term("a", "a")), new PhraseQuery.Builder().build()).hasPhrase()); - } - - public void testToString() { - assertEquals("Graph(hasBoolean=false, hasPhrase=false)", new GraphQuery().toString()); - assertEquals("Graph(a:a, a:b, hasBoolean=true, hasPhrase=false)", - new GraphQuery(new TermQuery(new Term("a", "a")), - new BooleanQuery.Builder().add(new TermQuery(new Term("a", "b")), BooleanClause.Occur.SHOULD) - .build()).toString()); - assertEquals("Graph(a:\"a b\", a:b, hasBoolean=true, hasPhrase=true)", - new GraphQuery( - new PhraseQuery.Builder() - .add(new Term("a", "a")) - .add(new Term("a", "b")).build(), - new BooleanQuery.Builder().add(new TermQuery(new Term("a", "b")), BooleanClause.Occur.SHOULD) - .build()).toString()); - } - - public void testRewrite() throws IOException { - QueryUtils.checkEqual(new BooleanQuery.Builder().build(), new GraphQuery().rewrite(null)); - QueryUtils.checkEqual(new TermQuery(new Term("a", "a")), - new GraphQuery(new TermQuery(new Term("a", "a"))).rewrite(null)); - QueryUtils.checkEqual( - new BooleanQuery.Builder() - .add(new TermQuery(new Term("a", "a")), BooleanClause.Occur.SHOULD) - .add(new TermQuery(new Term("b", "b")), BooleanClause.Occur.SHOULD).build(), - new GraphQuery( - new TermQuery(new Term("a", "a")), - new TermQuery(new Term("b", "b")) - ).rewrite(null) - ); - } -} 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 a408a61b119..fc04c5ed286 100644 --- a/lucene/core/src/test/org/apache/lucene/util/TestQueryBuilder.java +++ b/lucene/core/src/test/org/apache/lucene/util/TestQueryBuilder.java @@ -31,7 +31,6 @@ import org.apache.lucene.analysis.tokenattributes.PositionIncrementAttribute; import org.apache.lucene.index.Term; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.GraphQuery; import org.apache.lucene.search.MultiPhraseQuery; import org.apache.lucene.search.PhraseQuery; import org.apache.lucene.search.Query; @@ -178,7 +177,12 @@ public class TestQueryBuilder extends LuceneTestCase { .add(new TermQuery(new Term("field", "pig")), BooleanClause.Occur.MUST) .build(); Query syn2 = new TermQuery(new Term("field", "cavy")); - GraphQuery expectedGraphQuery = new GraphQuery(syn1, syn2); + + BooleanQuery expectedGraphQuery = new BooleanQuery.Builder() + .add(syn1, BooleanClause.Occur.SHOULD) + .add(syn2, BooleanClause.Occur.SHOULD) + .build(); + QueryBuilder queryBuilder = new QueryBuilder(new MockSynonymAnalyzer()); assertEquals(expectedGraphQuery, queryBuilder.createBooleanQuery("field", "guinea pig", occur)); @@ -213,7 +217,10 @@ public class TestQueryBuilder extends LuceneTestCase { .add(new Term("field", "pig")) .build(); Query syn2 = new TermQuery(new Term("field", "cavy")); - GraphQuery expectedGraphQuery = new GraphQuery(syn1, syn2); + BooleanQuery expectedGraphQuery = new BooleanQuery.Builder() + .add(syn1, BooleanClause.Occur.SHOULD) + .add(syn2, BooleanClause.Occur.SHOULD) + .build(); QueryBuilder queryBuilder = new QueryBuilder(new MockSynonymAnalyzer()); queryBuilder.setAutoGenerateMultiTermSynonymsPhraseQuery(true); assertEquals(expectedGraphQuery, queryBuilder.createBooleanQuery("field", "guinea pig", occur)); 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 9b238d87eff..3cfa7d0b8bf 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 @@ -494,21 +494,6 @@ public abstract class QueryParserBase extends QueryBuilder implements CommonQuer if (slop != mpq.getSlop()) { query = new MultiPhraseQuery.Builder(mpq).setSlop(slop).build(); } - } else if (query instanceof GraphQuery && ((GraphQuery) query).hasPhrase()) { - // we have a graph query that has at least one phrase sub-query - // re-build and set slop on all phrase queries - List oldQueries = ((GraphQuery) query).getQueries(); - Query[] queries = new Query[oldQueries.size()]; - for (int i = 0; i < queries.length; i++) { - Query oldQuery = oldQueries.get(i); - if (oldQuery instanceof PhraseQuery) { - queries[i] = addSlopToPhrase((PhraseQuery) oldQuery, slop); - } else { - queries[i] = oldQuery; - } - } - - query = new GraphQuery(queries); } return query; diff --git a/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestMultiFieldQueryParser.java b/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestMultiFieldQueryParser.java index 9e1fb558b7c..7490e8cb608 100644 --- a/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestMultiFieldQueryParser.java +++ b/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestMultiFieldQueryParser.java @@ -351,8 +351,7 @@ public class TestMultiFieldQueryParser extends LuceneTestCase { assertEquals("Synonym(b:dog b:dogs) Synonym(t:dog t:dogs)", q.toString()); q = parser.parse("guinea pig"); assertFalse(parser.getSplitOnWhitespace()); - assertEquals("Graph(+b:guinea +b:pig, b:cavy, hasBoolean=true, hasPhrase=false) " - + "Graph(+t:guinea +t:pig, t:cavy, hasBoolean=true, hasPhrase=false)", q.toString()); + assertEquals("((+b:guinea +b:pig) (+t:guinea +t:pig)) (b:cavy t:cavy)", q.toString()); parser.setSplitOnWhitespace(true); q = parser.parse("guinea pig"); assertEquals("(b:guinea t:guinea) (b:pig t:pig)", q.toString()); 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 ec1f4769d29..e8533e0a11d 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 @@ -43,7 +43,6 @@ import org.apache.lucene.queryparser.util.QueryParserTestBase; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.BoostQuery; -import org.apache.lucene.search.GraphQuery; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.MultiPhraseQuery; @@ -522,7 +521,10 @@ public class TestQueryParser extends QueryParserTestBase { .add(new Term("field", "pig")) .build(); - GraphQuery graphQuery = new GraphQuery(guineaPig, cavy); + BooleanQuery graphQuery = new BooleanQuery.Builder() + .add(guineaPig, BooleanClause.Occur.SHOULD) + .add(cavy, BooleanClause.Occur.SHOULD) + .build(); assertEquals(graphQuery, dumb.parse("guinea pig")); // With the phrase operator, a multi-word synonym source will form span near queries. @@ -538,7 +540,10 @@ public class TestQueryParser extends QueryParserTestBase { // custom behavior, the synonyms are expanded, unless you use quote operator QueryParser smart = new SmartQueryParser(); smart.setSplitOnWhitespace(false); - graphQuery = new GraphQuery(guineaPig, cavy); + graphQuery = new BooleanQuery.Builder() + .add(guineaPig, BooleanClause.Occur.SHOULD) + .add(cavy, BooleanClause.Occur.SHOULD) + .build(); assertEquals(graphQuery, smart.parse("guinea pig")); assertEquals(phraseGuineaPig, smart.parse("\"guinea pig\"")); } @@ -611,30 +616,30 @@ public class TestQueryParser extends QueryParserTestBase { assertQueryEquals("guinea /pig/", a, "guinea /pig/"); // Operators should not interrupt multiword analysis if not don't associate - assertQueryEquals("(guinea pig)", a, "Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); - assertQueryEquals("+(guinea pig)", a, "+Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); - assertQueryEquals("-(guinea pig)", a, "-Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); - assertQueryEquals("!(guinea pig)", a, "-Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); - assertQueryEquals("NOT (guinea pig)", a, "-Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); - assertQueryEquals("(guinea pig)^2", a, "(Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false))^2.0"); + assertQueryEquals("(guinea pig)", a, "(+guinea +pig) cavy"); + assertQueryEquals("+(guinea pig)", a, "+((+guinea +pig) cavy)"); + assertQueryEquals("-(guinea pig)", a, "-((+guinea +pig) cavy)"); + assertQueryEquals("!(guinea pig)", a, "-((+guinea +pig) cavy)"); + assertQueryEquals("NOT (guinea pig)", a, "-((+guinea +pig) cavy)"); + assertQueryEquals("(guinea pig)^2", a, "((+guinea +pig) cavy)^2.0"); - assertQueryEquals("field:(guinea pig)", a, "Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); + assertQueryEquals("field:(guinea pig)", a, "(+guinea +pig) cavy"); - assertQueryEquals("+small guinea pig", a, "+small Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); - assertQueryEquals("-small guinea pig", a, "-small Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); - assertQueryEquals("!small guinea pig", a, "-small Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); - assertQueryEquals("NOT small guinea pig", a, "-small Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); - assertQueryEquals("small* guinea pig", a, "small* Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); - assertQueryEquals("small? guinea pig", a, "small? Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); - assertQueryEquals("\"small\" guinea pig", a, "small Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); + assertQueryEquals("+small guinea pig", a, "+small (+guinea +pig) cavy"); + assertQueryEquals("-small guinea pig", a, "-small (+guinea +pig) cavy"); + assertQueryEquals("!small guinea pig", a, "-small (+guinea +pig) cavy"); + assertQueryEquals("NOT small guinea pig", a, "-small (+guinea +pig) cavy"); + assertQueryEquals("small* guinea pig", a, "small* (+guinea +pig) cavy"); + assertQueryEquals("small? guinea pig", a, "small? (+guinea +pig) cavy"); + assertQueryEquals("\"small\" guinea pig", a, "small (+guinea +pig) cavy"); - assertQueryEquals("guinea pig +running", a, "Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false) +running"); - assertQueryEquals("guinea pig -running", a, "Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false) -running"); - assertQueryEquals("guinea pig !running", a, "Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false) -running"); - assertQueryEquals("guinea pig NOT running", a, "Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false) -running"); - assertQueryEquals("guinea pig running*", a, "Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false) running*"); - assertQueryEquals("guinea pig running?", a, "Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false) running?"); - assertQueryEquals("guinea pig \"running\"", a, "Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false) running"); + assertQueryEquals("guinea pig +running", a, "(+guinea +pig) cavy +running"); + assertQueryEquals("guinea pig -running", a, "(+guinea +pig) cavy -running"); + assertQueryEquals("guinea pig !running", a, "(+guinea +pig) cavy -running"); + assertQueryEquals("guinea pig NOT running", a, "(+guinea +pig) cavy -running"); + assertQueryEquals("guinea pig running*", a, "(+guinea +pig) cavy running*"); + assertQueryEquals("guinea pig running?", a, "(+guinea +pig) cavy running?"); + assertQueryEquals("guinea pig \"running\"", a, "(+guinea +pig) cavy running"); assertQueryEquals("\"guinea pig\"~2", a, "spanOr([spanNear([guinea, pig], 0, true), cavy])"); @@ -738,12 +743,15 @@ public class TestQueryParser extends QueryParserTestBase { synonym.add(pig, BooleanClause.Occur.MUST); BooleanQuery guineaPig = synonym.build(); - GraphQuery graphQuery = new GraphQuery(guineaPig, cavy); + BooleanQuery graphQuery = new BooleanQuery.Builder() + .add(guineaPig, BooleanClause.Occur.SHOULD) + .add(cavy, BooleanClause.Occur.SHOULD) + .build();; assertEquals(graphQuery, parser.parse("guinea pig")); boolean oldSplitOnWhitespace = splitOnWhitespace; splitOnWhitespace = QueryParser.DEFAULT_SPLIT_ON_WHITESPACE; - assertQueryEquals("guinea pig", new MockSynonymAnalyzer(), "Graph(+field:guinea +field:pig, field:cavy, hasBoolean=true, hasPhrase=false)"); + assertQueryEquals("guinea pig", new MockSynonymAnalyzer(), "(+guinea +pig) cavy"); splitOnWhitespace = oldSplitOnWhitespace; }