You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ji...@apache.org on 2017/06/16 14:03:37 UTC

lucene-solr:branch_6x: LUCENE-7878: Fix query builder to keep the SHOULD clause that wraps multi-word synonyms

Repository: lucene-solr
Updated Branches:
  refs/heads/branch_6x 45af5576a -> 6b9e10159


LUCENE-7878: Fix query builder to keep the SHOULD clause that wraps multi-word synonyms


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/6b9e1015
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/6b9e1015
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/6b9e1015

Branch: refs/heads/branch_6x
Commit: 6b9e10159121477887ef77d3522095e0368c8577
Parents: 45af557
Author: Jim Ferenczi <ji...@apache.org>
Authored: Fri Jun 16 14:42:46 2017 +0200
Committer: Jim Ferenczi <ji...@apache.org>
Committed: Fri Jun 16 16:00:52 2017 +0200

----------------------------------------------------------------------
 lucene/CHANGES.txt                              |  2 +
 .../org/apache/lucene/util/QueryBuilder.java    |  6 +-
 .../apache/lucene/util/TestQueryBuilder.java    | 28 ++++---
 .../classic/TestMultiFieldQueryParser.java      |  2 +-
 .../queryparser/classic/TestQueryParser.java    | 79 +++++++++++++-------
 .../solr/search/TestExtendedDismaxParser.java   | 12 +--
 .../apache/solr/search/TestSolrQueryParser.java |  4 +-
 7 files changed, 81 insertions(+), 52 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6b9e1015/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 2564546..fb66fdb 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -19,6 +19,8 @@ Bug Fixes
   that these points are visited in ascending order. The memory index doesn't do this and this can result in document
   with multiple points that should match to not match. (Martijn van Groningen)
 
+* LUCENE-7878: Fix query builder to keep the SHOULD clause that wraps multi-word synonyms. (Jim Ferenczi)
+
 Other
 
 * LUCENE-7800: Remove code that potentially rethrows checked exceptions 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6b9e1015/lucene/core/src/java/org/apache/lucene/util/QueryBuilder.java
----------------------------------------------------------------------
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 fc5f97a..2991129 100644
--- a/lucene/core/src/java/org/apache/lucene/util/QueryBuilder.java
+++ b/lucene/core/src/java/org/apache/lucene/util/QueryBuilder.java
@@ -539,11 +539,7 @@ public class QueryBuilder {
         builder.add(queryPos, operator);
       }
     }
-    BooleanQuery bq =  builder.build();
-    if (bq.clauses().size() == 1) {
-      return bq.clauses().get(0).getQuery();
-    }
-    return bq;
+    return builder.build();
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6b9e1015/lucene/core/src/test/org/apache/lucene/util/TestQueryBuilder.java
----------------------------------------------------------------------
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 a9d803b..fece166 100644
--- a/lucene/core/src/test/org/apache/lucene/util/TestQueryBuilder.java
+++ b/lucene/core/src/test/org/apache/lucene/util/TestQueryBuilder.java
@@ -178,32 +178,36 @@ public class TestQueryBuilder extends LuceneTestCase {
           .build();
       Query syn2 = new TermQuery(new Term("field", "cavy"));
 
-      BooleanQuery expectedGraphQuery = new BooleanQuery.Builder()
+      BooleanQuery synQuery = new BooleanQuery.Builder()
           .add(syn1, BooleanClause.Occur.SHOULD)
           .add(syn2, BooleanClause.Occur.SHOULD)
           .build();
 
+      BooleanQuery expectedGraphQuery = new BooleanQuery.Builder()
+          .add(synQuery, occur)
+          .build();
+
       QueryBuilder queryBuilder = new QueryBuilder(new MockSynonymAnalyzer());
       assertEquals(expectedGraphQuery, queryBuilder.createBooleanQuery("field", "guinea pig", occur));
 
       BooleanQuery expectedBooleanQuery = new BooleanQuery.Builder()
-          .add(expectedGraphQuery, occur)
+          .add(synQuery, occur)
           .add(new TermQuery(new Term("field", "story")), occur)
           .build();
       assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "guinea pig story", occur));
 
       expectedBooleanQuery = new BooleanQuery.Builder()
           .add(new TermQuery(new Term("field", "the")), occur)
-          .add(expectedGraphQuery, occur)
+          .add(synQuery, occur)
           .add(new TermQuery(new Term("field", "story")), occur)
           .build();
       assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "the guinea pig story", occur));
 
       expectedBooleanQuery = new BooleanQuery.Builder()
           .add(new TermQuery(new Term("field", "the")), occur)
-          .add(expectedGraphQuery, occur)
+          .add(synQuery, occur)
           .add(new TermQuery(new Term("field", "story")), occur)
-          .add(expectedGraphQuery, occur)
+          .add(synQuery, occur)
           .build();
       assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "the guinea pig story guinea pig", occur));
     }
@@ -217,32 +221,36 @@ public class TestQueryBuilder extends LuceneTestCase {
           .add(new Term("field", "pig"))
           .build();
       Query syn2 = new TermQuery(new Term("field", "cavy"));
-      BooleanQuery expectedGraphQuery = new BooleanQuery.Builder()
+
+      BooleanQuery synQuery = new BooleanQuery.Builder()
           .add(syn1, BooleanClause.Occur.SHOULD)
           .add(syn2, BooleanClause.Occur.SHOULD)
           .build();
+      BooleanQuery expectedGraphQuery = new BooleanQuery.Builder()
+          .add(synQuery, occur)
+          .build();
       QueryBuilder queryBuilder = new QueryBuilder(new MockSynonymAnalyzer());
       queryBuilder.setAutoGenerateMultiTermSynonymsPhraseQuery(true);
       assertEquals(expectedGraphQuery, queryBuilder.createBooleanQuery("field", "guinea pig", occur));
 
       BooleanQuery expectedBooleanQuery = new BooleanQuery.Builder()
-          .add(expectedGraphQuery, occur)
+          .add(synQuery, occur)
           .add(new TermQuery(new Term("field", "story")), occur)
           .build();
       assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "guinea pig story", occur));
 
       expectedBooleanQuery = new BooleanQuery.Builder()
           .add(new TermQuery(new Term("field", "the")), occur)
-          .add(expectedGraphQuery, occur)
+          .add(synQuery, occur)
           .add(new TermQuery(new Term("field", "story")), occur)
           .build();
       assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "the guinea pig story", occur));
 
       expectedBooleanQuery = new BooleanQuery.Builder()
           .add(new TermQuery(new Term("field", "the")), occur)
-          .add(expectedGraphQuery, occur)
+          .add(synQuery, occur)
           .add(new TermQuery(new Term("field", "story")), occur)
-          .add(expectedGraphQuery, occur)
+          .add(synQuery, occur)
           .build();
       assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "the guinea pig story guinea pig", occur));
     }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6b9e1015/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestMultiFieldQueryParser.java
----------------------------------------------------------------------
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 4154e0c..0edede2 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
@@ -356,7 +356,7 @@ public class TestMultiFieldQueryParser extends LuceneTestCase {
     parser.setSplitOnWhitespace(false);
     q = parser.parse("guinea pig");
     assertFalse(parser.getSplitOnWhitespace());
-    assertEquals("((+b:guinea +b:pig) (+t:guinea +t:pig)) (b:cavy t:cavy)", q.toString());
+    assertEquals("((+b:guinea +b:pig) b:cavy) ((+t:guinea +t:pig) t:cavy)", q.toString());
     parser.setSplitOnWhitespace(true);
     q = parser.parse("guinea pig");
     assertEquals("(b:guinea t:guinea) (b:pig t:pig)", q.toString());

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6b9e1015/lucene/queryparser/src/test/org/apache/lucene/queryparser/classic/TestQueryParser.java
----------------------------------------------------------------------
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 c719815..4d06f2d 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
@@ -518,8 +518,10 @@ public class TestQueryParser extends QueryParserTestBase {
         .build();
 
     BooleanQuery graphQuery = new BooleanQuery.Builder()
-        .add(guineaPig, BooleanClause.Occur.SHOULD)
-        .add(cavy, BooleanClause.Occur.SHOULD)
+        .add(new BooleanQuery.Builder()
+            .add(guineaPig, BooleanClause.Occur.SHOULD)
+            .add(cavy, BooleanClause.Occur.SHOULD)
+            .build(), BooleanClause.Occur.SHOULD)
         .build();
     assertEquals(graphQuery, dumb.parse("guinea pig"));
 
@@ -537,11 +539,32 @@ public class TestQueryParser extends QueryParserTestBase {
     QueryParser smart = new SmartQueryParser();
     smart.setSplitOnWhitespace(false);
     graphQuery = new BooleanQuery.Builder()
-        .add(guineaPig, BooleanClause.Occur.SHOULD)
-        .add(cavy, BooleanClause.Occur.SHOULD)
+        .add(new BooleanQuery.Builder()
+            .add(guineaPig, BooleanClause.Occur.SHOULD)
+            .add(cavy, BooleanClause.Occur.SHOULD)
+            .build(), BooleanClause.Occur.SHOULD)
         .build();
     assertEquals(graphQuery, smart.parse("guinea pig"));
     assertEquals(phraseGuineaPig, smart.parse("\"guinea pig\""));
+
+    // with the AND operator
+    dumb.setDefaultOperator(Operator.AND);
+    BooleanQuery graphAndQuery = new BooleanQuery.Builder()
+        .add(new BooleanQuery.Builder()
+            .add(guineaPig, BooleanClause.Occur.SHOULD)
+            .add(cavy, BooleanClause.Occur.SHOULD)
+            .build(), BooleanClause.Occur.MUST)
+        .build();
+    assertEquals(graphAndQuery, dumb.parse("guinea pig"));
+
+    graphAndQuery = new BooleanQuery.Builder()
+        .add(new BooleanQuery.Builder()
+            .add(guineaPig, BooleanClause.Occur.SHOULD)
+            .add(cavy, BooleanClause.Occur.SHOULD)
+            .build(), BooleanClause.Occur.MUST)
+        .add(cavy, BooleanClause.Occur.MUST)
+        .build();
+    assertEquals(graphAndQuery, dumb.parse("guinea pig cavy"));
   }
 
   public void testEnableGraphQueries() throws Exception {
@@ -612,30 +635,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, "(+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, "(+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("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, "(+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)", 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, "((+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("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, "((+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])");
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6b9e1015/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java b/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
index f466329..055814b 100644
--- a/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
+++ b/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
@@ -1774,7 +1774,7 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
     try (SolrQueryRequest req = req(sowFalseParams)) {
       QParser qParser = QParser.getParser("text:grackle", "edismax", req); // "text" has autoGeneratePhraseQueries="true"
       Query q = qParser.getQuery();
-      assertEquals("+(text:\"crow blackbird\" text:grackl)", q.toString());
+      assertEquals("+((text:\"crow blackbird\" text:grackl))", q.toString());
     }
     for (SolrParams params : Arrays.asList(noSowParams, sowTrueParams)) {
       try (SolrQueryRequest req = req(params)) {
@@ -1787,7 +1787,7 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
       try (SolrQueryRequest req = req(params)) {
         QParser qParser = QParser.getParser("text_sw:grackle", "edismax", req); // "text_sw" doesn't specify autoGeneratePhraseQueries => default false
         Query q = qParser.getQuery();
-        assertEquals("+((+text_sw:crow +text_sw:blackbird) text_sw:grackl)", q.toString());
+        assertEquals("+(((+text_sw:crow +text_sw:blackbird) text_sw:grackl))", q.toString());
       }
     }
 
@@ -1796,8 +1796,8 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
     try (SolrQueryRequest req = req(sowFalseParams)) {
       QParser qParser = QParser.getParser("grackle", "edismax", req);
       Query q = qParser.getQuery();
-      assertEquals("+((text:\"crow blackbird\" text:grackl)"
-              + " | ((+text_sw:crow +text_sw:blackbird) text_sw:grackl))",
+      assertEquals("+(((text:\"crow blackbird\" text:grackl))"
+              + " | (((+text_sw:crow +text_sw:blackbird) text_sw:grackl)))",
           q.toString());
 
       qParser = QParser.getParser("grackle wi fi", "edismax", req);
@@ -1812,13 +1812,13 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
         QParser qParser = QParser.getParser("grackle", "edismax", req);
         Query q = qParser.getQuery();
         assertEquals("+(spanOr([spanNear([text:crow, text:blackbird], 0, true), text:grackl])"
-                + " | ((+text_sw:crow +text_sw:blackbird) text_sw:grackl))",
+                + " | (((+text_sw:crow +text_sw:blackbird) text_sw:grackl)))",
             q.toString());
 
         qParser = QParser.getParser("grackle wi fi", "edismax", req);
         q = qParser.getQuery();
         assertEquals("+((spanOr([spanNear([text:crow, text:blackbird], 0, true), text:grackl])"
-            + " | ((+text_sw:crow +text_sw:blackbird) text_sw:grackl)) (text:wi | text_sw:wi) (text:fi | text_sw:fi))",
+                + " | (((+text_sw:crow +text_sw:blackbird) text_sw:grackl))) (text:wi | text_sw:wi) (text:fi | text_sw:fi))",
             q.toString());
       }
     }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6b9e1015/solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java b/solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java
index 59ffb38..e776e1e 100644
--- a/solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java
+++ b/solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java
@@ -981,7 +981,7 @@ public class TestSolrQueryParser extends SolrTestCaseJ4 {
       QParser qParser = QParser.getParser("text:grackle", req); // "text" has autoGeneratePhraseQueries="true"
       qParser.setParams(sowFalseParams);
       Query q = qParser.getQuery();
-      assertEquals("text:\"crow blackbird\" text:grackl", q.toString());
+      assertEquals("(text:\"crow blackbird\" text:grackl)", q.toString());
 
       for (SolrParams params : Arrays.asList(noSowParams, sowTrueParams)) {
         qParser = QParser.getParser("text:grackle", req);
@@ -994,7 +994,7 @@ public class TestSolrQueryParser extends SolrTestCaseJ4 {
         qParser = QParser.getParser("text_sw:grackle", req); // "text_sw" doesn't specify autoGeneratePhraseQueries => default false
         qParser.setParams(params);
         q = qParser.getQuery();
-        assertEquals("(+text_sw:crow +text_sw:blackbird) text_sw:grackl", q.toString());
+        assertEquals("((+text_sw:crow +text_sw:blackbird) text_sw:grackl)", q.toString());
       }
     }
   }