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 12:43:05 UTC
lucene-solr:master: LUCENE-7878: Fix query builder to keep the SHOULD
clause that wraps multi-word synonyms
Repository: lucene-solr
Updated Branches:
refs/heads/master 1744fa254 -> 0c683305a
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/0c683305
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/0c683305
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/0c683305
Branch: refs/heads/master
Commit: 0c683305a4aae59869b4b0524d10a4cdf48ec036
Parents: 1744fa2
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 14:42:46 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 | 89 +++++++++++++-------
.../solr/search/TestExtendedDismaxParser.java | 12 +--
.../apache/solr/search/TestSolrQueryParser.java | 4 +-
7 files changed, 87 insertions(+), 56 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/0c683305/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 96cfb3a..f11e8af 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -144,6 +144,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/0c683305/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 9fb474a..f077bfd 100644
--- a/lucene/core/src/java/org/apache/lucene/util/QueryBuilder.java
+++ b/lucene/core/src/java/org/apache/lucene/util/QueryBuilder.java
@@ -538,11 +538,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/0c683305/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/0c683305/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 4c28e8f..cc46599 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,7 +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("((+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/0c683305/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 3450794..17107fc 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
@@ -522,8 +522,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"));
@@ -541,11 +543,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 {
@@ -616,30 +639,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])");
@@ -744,14 +767,16 @@ public class TestQueryParser extends QueryParserTestBase {
BooleanQuery guineaPig = synonym.build();
BooleanQuery graphQuery = new BooleanQuery.Builder()
- .add(guineaPig, BooleanClause.Occur.SHOULD)
- .add(cavy, BooleanClause.Occur.SHOULD)
- .build();;
+ .add(new BooleanQuery.Builder()
+ .add(guineaPig, BooleanClause.Occur.SHOULD)
+ .add(cavy, BooleanClause.Occur.SHOULD)
+ .build(), BooleanClause.Occur.SHOULD)
+ .build();
assertEquals(graphQuery, parser.parse("guinea pig"));
boolean oldSplitOnWhitespace = splitOnWhitespace;
splitOnWhitespace = QueryParser.DEFAULT_SPLIT_ON_WHITESPACE;
- assertQueryEquals("guinea pig", new MockSynonymAnalyzer(), "(+guinea +pig) cavy");
+ assertQueryEquals("guinea pig", new MockSynonymAnalyzer(), "((+guinea +pig) cavy)");
splitOnWhitespace = oldSplitOnWhitespace;
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/0c683305/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 d416b22..46bebb3 100644
--- a/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
+++ b/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
@@ -1787,7 +1787,7 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
try (SolrQueryRequest req = req(params)) {
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());
}
}
try (SolrQueryRequest req = req(sowTrueParams)) {
@@ -1799,7 +1799,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());
}
}
@@ -1809,8 +1809,8 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
try (SolrQueryRequest req = req(params)) {
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);
@@ -1825,13 +1825,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/0c683305/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 21bf2c7..6db4800 100644
--- a/solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java
+++ b/solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java
@@ -1007,7 +1007,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());
}
QParser qParser = QParser.getParser("text:grackle", req);
@@ -1019,7 +1019,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());
}
}
}