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:10:50 UTC

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

Repository: lucene-solr
Updated Branches:
  refs/heads/branch_6_6 ff18df65a -> 9f89811e6


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/9f89811e
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/9f89811e
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/9f89811e

Branch: refs/heads/branch_6_6
Commit: 9f89811e6524ba21777a256f478744dc4a40a685
Parents: ff18df6
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:10:36 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/9f89811e/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 89f124f..7ce9377 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -11,6 +11,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)
+
 ======================= Lucene 6.6.0 =======================
 
 New Features

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9f89811e/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/9f89811e/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/9f89811e/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/9f89811e/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/9f89811e/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/9f89811e/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 e1372d8..6d80555 100644
--- a/solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java
+++ b/solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java
@@ -980,7 +980,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);
@@ -993,7 +993,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());
       }
     }
   }