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

This commit is contained in:
Jim Ferenczi 2017-06-16 14:42:46 +02:00
parent 1744fa254a
commit 0c683305a4
7 changed files with 84 additions and 53 deletions

View File

@ -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 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) 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 Other
* LUCENE-7800: Remove code that potentially rethrows checked exceptions * LUCENE-7800: Remove code that potentially rethrows checked exceptions

View File

@ -538,11 +538,7 @@ public class QueryBuilder {
builder.add(queryPos, operator); builder.add(queryPos, operator);
} }
} }
BooleanQuery bq = builder.build(); return builder.build();
if (bq.clauses().size() == 1) {
return bq.clauses().get(0).getQuery();
}
return bq;
} }
/** /**

View File

@ -178,32 +178,36 @@ public class TestQueryBuilder extends LuceneTestCase {
.build(); .build();
Query syn2 = new TermQuery(new Term("field", "cavy")); Query syn2 = new TermQuery(new Term("field", "cavy"));
BooleanQuery expectedGraphQuery = new BooleanQuery.Builder() BooleanQuery synQuery = new BooleanQuery.Builder()
.add(syn1, BooleanClause.Occur.SHOULD) .add(syn1, BooleanClause.Occur.SHOULD)
.add(syn2, BooleanClause.Occur.SHOULD) .add(syn2, BooleanClause.Occur.SHOULD)
.build(); .build();
BooleanQuery expectedGraphQuery = new BooleanQuery.Builder()
.add(synQuery, occur)
.build();
QueryBuilder queryBuilder = new QueryBuilder(new MockSynonymAnalyzer()); QueryBuilder queryBuilder = new QueryBuilder(new MockSynonymAnalyzer());
assertEquals(expectedGraphQuery, queryBuilder.createBooleanQuery("field", "guinea pig", occur)); assertEquals(expectedGraphQuery, queryBuilder.createBooleanQuery("field", "guinea pig", occur));
BooleanQuery expectedBooleanQuery = new BooleanQuery.Builder() BooleanQuery expectedBooleanQuery = new BooleanQuery.Builder()
.add(expectedGraphQuery, occur) .add(synQuery, occur)
.add(new TermQuery(new Term("field", "story")), occur) .add(new TermQuery(new Term("field", "story")), occur)
.build(); .build();
assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "guinea pig story", occur)); assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "guinea pig story", occur));
expectedBooleanQuery = new BooleanQuery.Builder() expectedBooleanQuery = new BooleanQuery.Builder()
.add(new TermQuery(new Term("field", "the")), occur) .add(new TermQuery(new Term("field", "the")), occur)
.add(expectedGraphQuery, occur) .add(synQuery, occur)
.add(new TermQuery(new Term("field", "story")), occur) .add(new TermQuery(new Term("field", "story")), occur)
.build(); .build();
assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "the guinea pig story", occur)); assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "the guinea pig story", occur));
expectedBooleanQuery = new BooleanQuery.Builder() expectedBooleanQuery = new BooleanQuery.Builder()
.add(new TermQuery(new Term("field", "the")), occur) .add(new TermQuery(new Term("field", "the")), occur)
.add(expectedGraphQuery, occur) .add(synQuery, occur)
.add(new TermQuery(new Term("field", "story")), occur) .add(new TermQuery(new Term("field", "story")), occur)
.add(expectedGraphQuery, occur) .add(synQuery, occur)
.build(); .build();
assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "the guinea pig story guinea pig", occur)); 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")) .add(new Term("field", "pig"))
.build(); .build();
Query syn2 = new TermQuery(new Term("field", "cavy")); Query syn2 = new TermQuery(new Term("field", "cavy"));
BooleanQuery expectedGraphQuery = new BooleanQuery.Builder()
BooleanQuery synQuery = new BooleanQuery.Builder()
.add(syn1, BooleanClause.Occur.SHOULD) .add(syn1, BooleanClause.Occur.SHOULD)
.add(syn2, BooleanClause.Occur.SHOULD) .add(syn2, BooleanClause.Occur.SHOULD)
.build(); .build();
BooleanQuery expectedGraphQuery = new BooleanQuery.Builder()
.add(synQuery, occur)
.build();
QueryBuilder queryBuilder = new QueryBuilder(new MockSynonymAnalyzer()); QueryBuilder queryBuilder = new QueryBuilder(new MockSynonymAnalyzer());
queryBuilder.setAutoGenerateMultiTermSynonymsPhraseQuery(true); queryBuilder.setAutoGenerateMultiTermSynonymsPhraseQuery(true);
assertEquals(expectedGraphQuery, queryBuilder.createBooleanQuery("field", "guinea pig", occur)); assertEquals(expectedGraphQuery, queryBuilder.createBooleanQuery("field", "guinea pig", occur));
BooleanQuery expectedBooleanQuery = new BooleanQuery.Builder() BooleanQuery expectedBooleanQuery = new BooleanQuery.Builder()
.add(expectedGraphQuery, occur) .add(synQuery, occur)
.add(new TermQuery(new Term("field", "story")), occur) .add(new TermQuery(new Term("field", "story")), occur)
.build(); .build();
assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "guinea pig story", occur)); assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "guinea pig story", occur));
expectedBooleanQuery = new BooleanQuery.Builder() expectedBooleanQuery = new BooleanQuery.Builder()
.add(new TermQuery(new Term("field", "the")), occur) .add(new TermQuery(new Term("field", "the")), occur)
.add(expectedGraphQuery, occur) .add(synQuery, occur)
.add(new TermQuery(new Term("field", "story")), occur) .add(new TermQuery(new Term("field", "story")), occur)
.build(); .build();
assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "the guinea pig story", occur)); assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "the guinea pig story", occur));
expectedBooleanQuery = new BooleanQuery.Builder() expectedBooleanQuery = new BooleanQuery.Builder()
.add(new TermQuery(new Term("field", "the")), occur) .add(new TermQuery(new Term("field", "the")), occur)
.add(expectedGraphQuery, occur) .add(synQuery, occur)
.add(new TermQuery(new Term("field", "story")), occur) .add(new TermQuery(new Term("field", "story")), occur)
.add(expectedGraphQuery, occur) .add(synQuery, occur)
.build(); .build();
assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "the guinea pig story guinea pig", occur)); assertEquals(expectedBooleanQuery, queryBuilder.createBooleanQuery("field", "the guinea pig story guinea pig", occur));
} }

View File

@ -351,7 +351,7 @@ public class TestMultiFieldQueryParser extends LuceneTestCase {
assertEquals("Synonym(b:dog b:dogs) Synonym(t:dog t:dogs)", q.toString()); assertEquals("Synonym(b:dog b:dogs) Synonym(t:dog t:dogs)", q.toString());
q = parser.parse("guinea pig"); q = parser.parse("guinea pig");
assertFalse(parser.getSplitOnWhitespace()); 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); parser.setSplitOnWhitespace(true);
q = parser.parse("guinea pig"); q = parser.parse("guinea pig");
assertEquals("(b:guinea t:guinea) (b:pig t:pig)", q.toString()); assertEquals("(b:guinea t:guinea) (b:pig t:pig)", q.toString());

View File

@ -522,8 +522,10 @@ public class TestQueryParser extends QueryParserTestBase {
.build(); .build();
BooleanQuery graphQuery = new BooleanQuery.Builder() BooleanQuery graphQuery = new BooleanQuery.Builder()
.add(guineaPig, BooleanClause.Occur.SHOULD) .add(new BooleanQuery.Builder()
.add(cavy, BooleanClause.Occur.SHOULD) .add(guineaPig, BooleanClause.Occur.SHOULD)
.add(cavy, BooleanClause.Occur.SHOULD)
.build(), BooleanClause.Occur.SHOULD)
.build(); .build();
assertEquals(graphQuery, dumb.parse("guinea pig")); assertEquals(graphQuery, dumb.parse("guinea pig"));
@ -541,11 +543,32 @@ public class TestQueryParser extends QueryParserTestBase {
QueryParser smart = new SmartQueryParser(); QueryParser smart = new SmartQueryParser();
smart.setSplitOnWhitespace(false); smart.setSplitOnWhitespace(false);
graphQuery = new BooleanQuery.Builder() graphQuery = new BooleanQuery.Builder()
.add(guineaPig, BooleanClause.Occur.SHOULD) .add(new BooleanQuery.Builder()
.add(cavy, BooleanClause.Occur.SHOULD) .add(guineaPig, BooleanClause.Occur.SHOULD)
.add(cavy, BooleanClause.Occur.SHOULD)
.build(), BooleanClause.Occur.SHOULD)
.build(); .build();
assertEquals(graphQuery, smart.parse("guinea pig")); assertEquals(graphQuery, smart.parse("guinea pig"));
assertEquals(phraseGuineaPig, 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 { public void testEnableGraphQueries() throws Exception {
@ -616,30 +639,30 @@ public class TestQueryParser extends QueryParserTestBase {
assertQueryEquals("guinea /pig/", a, "guinea /pig/"); assertQueryEquals("guinea /pig/", a, "guinea /pig/");
// Operators should not interrupt multiword analysis if not don't associate // 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("-(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("NOT (guinea pig)", a, "-(((+guinea +pig) cavy))");
assertQueryEquals("(guinea pig)^2", a, "((+guinea +pig) cavy)^2.0"); assertQueryEquals("(guinea pig)^2", a, "(((+guinea +pig) cavy))^2.0");
assertQueryEquals("field:(guinea pig)", a, "(+guinea +pig) cavy"); 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("-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("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("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 -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 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 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])"); 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 guineaPig = synonym.build();
BooleanQuery graphQuery = new BooleanQuery.Builder() BooleanQuery graphQuery = new BooleanQuery.Builder()
.add(guineaPig, BooleanClause.Occur.SHOULD) .add(new BooleanQuery.Builder()
.add(cavy, BooleanClause.Occur.SHOULD) .add(guineaPig, BooleanClause.Occur.SHOULD)
.build();; .add(cavy, BooleanClause.Occur.SHOULD)
.build(), BooleanClause.Occur.SHOULD)
.build();
assertEquals(graphQuery, parser.parse("guinea pig")); assertEquals(graphQuery, parser.parse("guinea pig"));
boolean oldSplitOnWhitespace = splitOnWhitespace; boolean oldSplitOnWhitespace = splitOnWhitespace;
splitOnWhitespace = QueryParser.DEFAULT_SPLIT_ON_WHITESPACE; splitOnWhitespace = QueryParser.DEFAULT_SPLIT_ON_WHITESPACE;
assertQueryEquals("guinea pig", new MockSynonymAnalyzer(), "(+guinea +pig) cavy"); assertQueryEquals("guinea pig", new MockSynonymAnalyzer(), "((+guinea +pig) cavy)");
splitOnWhitespace = oldSplitOnWhitespace; splitOnWhitespace = oldSplitOnWhitespace;
} }

View File

@ -1787,7 +1787,7 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
try (SolrQueryRequest req = req(params)) { try (SolrQueryRequest req = req(params)) {
QParser qParser = QParser.getParser("text:grackle", "edismax", req); // "text" has autoGeneratePhraseQueries="true" QParser qParser = QParser.getParser("text:grackle", "edismax", req); // "text" has autoGeneratePhraseQueries="true"
Query q = qParser.getQuery(); 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)) { try (SolrQueryRequest req = req(sowTrueParams)) {
@ -1799,7 +1799,7 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
try (SolrQueryRequest req = req(params)) { try (SolrQueryRequest req = req(params)) {
QParser qParser = QParser.getParser("text_sw:grackle", "edismax", req); // "text_sw" doesn't specify autoGeneratePhraseQueries => default false QParser qParser = QParser.getParser("text_sw:grackle", "edismax", req); // "text_sw" doesn't specify autoGeneratePhraseQueries => default false
Query q = qParser.getQuery(); 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)) { try (SolrQueryRequest req = req(params)) {
QParser qParser = QParser.getParser("grackle", "edismax", req); QParser qParser = QParser.getParser("grackle", "edismax", req);
Query q = qParser.getQuery(); Query q = qParser.getQuery();
assertEquals("+((text:\"crow blackbird\" text:grackl)" assertEquals("+(((text:\"crow blackbird\" text:grackl))"
+ " | ((+text_sw:crow +text_sw:blackbird) text_sw:grackl))", + " | (((+text_sw:crow +text_sw:blackbird) text_sw:grackl)))",
q.toString()); q.toString());
qParser = QParser.getParser("grackle wi fi", "edismax", req); qParser = QParser.getParser("grackle wi fi", "edismax", req);
@ -1825,13 +1825,13 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
QParser qParser = QParser.getParser("grackle", "edismax", req); QParser qParser = QParser.getParser("grackle", "edismax", req);
Query q = qParser.getQuery(); Query q = qParser.getQuery();
assertEquals("+(spanOr([spanNear([text:crow, text:blackbird], 0, true), text:grackl])" 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()); q.toString());
qParser = QParser.getParser("grackle wi fi", "edismax", req); qParser = QParser.getParser("grackle wi fi", "edismax", req);
q = qParser.getQuery(); q = qParser.getQuery();
assertEquals("+((spanOr([spanNear([text:crow, text:blackbird], 0, true), text:grackl])" 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()); q.toString());
} }
} }

View File

@ -1007,7 +1007,7 @@ public class TestSolrQueryParser extends SolrTestCaseJ4 {
QParser qParser = QParser.getParser("text:grackle", req); // "text" has autoGeneratePhraseQueries="true" QParser qParser = QParser.getParser("text:grackle", req); // "text" has autoGeneratePhraseQueries="true"
qParser.setParams(sowFalseParams); qParser.setParams(sowFalseParams);
Query q = qParser.getQuery(); 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); 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 = QParser.getParser("text_sw:grackle", req); // "text_sw" doesn't specify autoGeneratePhraseQueries => default false
qParser.setParams(params); qParser.setParams(params);
q = qParser.getQuery(); 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());
} }
} }
} }