LUCENE-7702: Remove GraphQuery in favor of simple boolean query.

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)

lucene/core/src/java/org/apache/lucene/search/GraphQuery.java

@@ -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<Query> 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});
-  }
-}

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();
+    }
   }
   
   /**

lucene/core/src/test/org/apache/lucene/search/TestGraphQuery.java

@@ -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)
-    );
-  }
-}

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));

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<Query> 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;

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());

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;
   }