You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by yo...@apache.org on 2014/04/24 20:12:15 UTC
svn commit: r1589812 - in /lucene/dev/trunk: lucene/
lucene/queryparser/src/java/org/apache/lucene/queryparser/complexPhrase/
lucene/queryparser/src/test/org/apache/lucene/queryparser/complexPhrase/
solr/core/src/test/org/apache/solr/search/
Author: yonik
Date: Thu Apr 24 18:12:15 2014
New Revision: 1589812
URL: http://svn.apache.org/r1589812
Log:
SOLR-6011: ComplexPhraseQuery hashCode/equals fix for inOrder
Modified:
lucene/dev/trunk/lucene/CHANGES.txt
lucene/dev/trunk/lucene/queryparser/src/java/org/apache/lucene/queryparser/complexPhrase/ComplexPhraseQueryParser.java
lucene/dev/trunk/lucene/queryparser/src/test/org/apache/lucene/queryparser/complexPhrase/TestComplexPhraseQuery.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestComplexPhraseQParserPlugin.java
Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1589812&r1=1589811&r2=1589812&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Thu Apr 24 18:12:15 2014
@@ -319,6 +319,10 @@ Bug fixes
* LUCENE-5624: Ensure NativeFSLockFactory does not leak file handles if it is unable
to obtain the lock. (Uwe Schindler, Robert Muir)
+* SOLR-6011: ComplexPhraseQueryParser produced Query objects that did not correctly
+ implement hashCode and equals (inOrder was ignored), causing issues for any
+ system using Query objects as keys. (yonik)
+
Test Framework
* LUCENE-5592: Incorrectly reported uncloseable files. (Dawid Weiss)
Modified: lucene/dev/trunk/lucene/queryparser/src/java/org/apache/lucene/queryparser/complexPhrase/ComplexPhraseQueryParser.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/queryparser/src/java/org/apache/lucene/queryparser/complexPhrase/ComplexPhraseQueryParser.java?rev=1589812&r1=1589811&r2=1589812&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/queryparser/src/java/org/apache/lucene/queryparser/complexPhrase/ComplexPhraseQueryParser.java (original)
+++ lucene/dev/trunk/lucene/queryparser/src/java/org/apache/lucene/queryparser/complexPhrase/ComplexPhraseQueryParser.java Thu Apr 24 18:12:15 2014
@@ -208,11 +208,11 @@ public class ComplexPhraseQueryParser ex
*/
static class ComplexPhraseQuery extends Query {
- String field;
+ final String field;
- String phrasedQueryStringContents;
+ final String phrasedQueryStringContents;
- int slopFactor;
+ final int slopFactor;
private final boolean inOrder;
@@ -394,6 +394,7 @@ public class ComplexPhraseQueryParser ex
+ ((phrasedQueryStringContents == null) ? 0
: phrasedQueryStringContents.hashCode());
result = prime * result + slopFactor;
+ result = prime * result + (inOrder ? 1 : 0);
return result;
}
@@ -422,7 +423,7 @@ public class ComplexPhraseQueryParser ex
return false;
if (slopFactor != other.slopFactor)
return false;
- return true;
+ return inOrder == other.inOrder;
}
}
}
Modified: lucene/dev/trunk/lucene/queryparser/src/test/org/apache/lucene/queryparser/complexPhrase/TestComplexPhraseQuery.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/queryparser/src/test/org/apache/lucene/queryparser/complexPhrase/TestComplexPhraseQuery.java?rev=1589812&r1=1589811&r2=1589812&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/queryparser/src/test/org/apache/lucene/queryparser/complexPhrase/TestComplexPhraseQuery.java (original)
+++ lucene/dev/trunk/lucene/queryparser/src/test/org/apache/lucene/queryparser/complexPhrase/TestComplexPhraseQuery.java Thu Apr 24 18:12:15 2014
@@ -139,6 +139,31 @@ public class TestComplexPhraseQuery exte
checkMatches("+role:developer +name:jack*", "");
checkMatches("name:\"john smith\"~2 AND role:designer AND id:3", "3");
}
+
+ public void testHashcodeEquals() throws Exception {
+ ComplexPhraseQueryParser qp = new ComplexPhraseQueryParser(TEST_VERSION_CURRENT, defaultFieldName, analyzer);
+ qp.setInOrder(true);
+ qp.setFuzzyPrefixLength(1);
+
+ String qString = "\"aaa* bbb*\"";
+
+ Query q = qp.parse(qString);
+ Query q2 = qp.parse(qString);
+
+ assertEquals(q.hashCode(), q2.hashCode());
+ assertEquals(q, q2);
+
+ qp.setInOrder(false); // SOLR-6011
+
+ q2 = qp.parse(qString);
+
+ // although the general contract of hashCode can't guarantee different values, if we only change one thing
+ // about a single query, it normally should result in a different value (and will with the current
+ // implementation in ComplexPhraseQuery)
+ assertTrue(q.hashCode() != q2.hashCode());
+ assertTrue(!q.equals(q2));
+ assertTrue(!q2.equals(q));
+ }
@Override
public void setUp() throws Exception {
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestComplexPhraseQParserPlugin.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestComplexPhraseQParserPlugin.java?rev=1589812&r1=1589811&r2=1589812&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestComplexPhraseQParserPlugin.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestComplexPhraseQParserPlugin.java Thu Apr 24 18:12:15 2014
@@ -30,7 +30,7 @@ public class TestComplexPhraseQParserPlu
@BeforeClass
public static void beforeClass() throws Exception {
- initCore("solrconfig-query-parser-init.xml","schema-complex-phrase.xml");
+ initCore("solrconfig.xml","schema15.xml");
}
@Override
@@ -54,43 +54,43 @@ public class TestComplexPhraseQParserPlu
assertQ(req("q", "{!complexphrase} \"john smith\"")
, "//result[@numFound='1']"
- , "//doc[./int[@name='id']='1']"
+ , "//doc[./str[@name='id']='1']"
);
assertQ(req("q", "{!complexphrase} \"j* smyth~\"")
, "//result[@numFound='2']"
- , "//doc[./int[@name='id']='1']"
- , "//doc[./int[@name='id']='2']"
+ , "//doc[./str[@name='id']='1']"
+ , "//doc[./str[@name='id']='2']"
);
assertQ(req("q", "{!complexphrase} \"(jo* -john) smith\"")
, "//result[@numFound='1']"
- , "//doc[./int[@name='id']='2']"
+ , "//doc[./str[@name='id']='2']"
);
assertQ(req("q", "{!complexphrase} \"jo* smith\"~2")
, "//result[@numFound='3']"
- , "//doc[./int[@name='id']='1']"
- , "//doc[./int[@name='id']='2']"
- , "//doc[./int[@name='id']='3']"
+ , "//doc[./str[@name='id']='1']"
+ , "//doc[./str[@name='id']='2']"
+ , "//doc[./str[@name='id']='3']"
);
assertQ(req("q", "{!complexphrase} \"jo* [sma TO smz]\"")
, "//result[@numFound='2']"
- , "//doc[./int[@name='id']='1']"
- , "//doc[./int[@name='id']='2']"
+ , "//doc[./str[@name='id']='1']"
+ , "//doc[./str[@name='id']='2']"
);
assertQ(req("q", "{!complexphrase} \"john\"")
, "//result[@numFound='2']"
- , "//doc[./int[@name='id']='1']"
- , "//doc[./int[@name='id']='3']"
+ , "//doc[./str[@name='id']='1']"
+ , "//doc[./str[@name='id']='3']"
);
assertQ(req("q", "{!complexphrase} \"(john johathon) smith\"")
, "//result[@numFound='2']"
- , "//doc[./int[@name='id']='1']"
- , "//doc[./int[@name='id']='2']"
+ , "//doc[./str[@name='id']='1']"
+ , "//doc[./str[@name='id']='2']"
);
}
@@ -113,55 +113,55 @@ public class TestComplexPhraseQParserPlu
assertQ("Simple multi-term still works",
sumLRF.makeRequest("name:\"john smith\""),
- "//doc[./int[@name='id']='1']",
+ "//doc[./str[@name='id']='1']",
"//result[@numFound='1']"
);
assertQ(req("q", "{!complexphrase} name:\"john smith\""),
- "//doc[./int[@name='id']='1']",
+ "//doc[./str[@name='id']='1']",
"//result[@numFound='1']"
);
assertQ("wildcards and fuzzies are OK in phrases",
sumLRF.makeRequest("name:\"j* smyth~\""),
- "//doc[./int[@name='id']='1']",
- "//doc[./int[@name='id']='2']",
+ "//doc[./str[@name='id']='1']",
+ "//doc[./str[@name='id']='2']",
"//result[@numFound='2']"
);
assertQ("boolean logic works",
sumLRF.makeRequest("name:\"(jo* -john) smith\""),
- "//doc[./int[@name='id']='2']",
+ "//doc[./str[@name='id']='2']",
"//result[@numFound='1']"
);
assertQ("position logic works",
sumLRF.makeRequest("name:\"jo* smith\"~2"),
- "//doc[./int[@name='id']='1']",
- "//doc[./int[@name='id']='2']",
- "//doc[./int[@name='id']='3']",
+ "//doc[./str[@name='id']='1']",
+ "//doc[./str[@name='id']='2']",
+ "//doc[./str[@name='id']='3']",
"//result[@numFound='3']"
);
assertQ("range queries supported",
sumLRF.makeRequest("name:\"jo* [sma TO smz]\""),
- "//doc[./int[@name='id']='1']",
- "//doc[./int[@name='id']='2']",
+ "//doc[./str[@name='id']='1']",
+ "//doc[./str[@name='id']='2']",
"//result[@numFound='2']"
);
assertQ("Simple single-term still works",
sumLRF.makeRequest("name:\"john\""),
- "//doc[./int[@name='id']='1']",
- "//doc[./int[@name='id']='3']",
+ "//doc[./str[@name='id']='1']",
+ "//doc[./str[@name='id']='3']",
"//result[@numFound='2']"
);
assertQ("OR inside phrase works",
sumLRF.makeRequest("name:\"(john johathon) smith\""),
- "//doc[./int[@name='id']='1']",
- "//doc[./int[@name='id']='2']",
+ "//doc[./str[@name='id']='1']",
+ "//doc[./str[@name='id']='2']",
"//result[@numFound='2']"
);
@@ -192,9 +192,9 @@ public class TestComplexPhraseQParserPlu
assertQ("range queries supported",
sumLRF.makeRequest("name:[sma TO smz]"),
- "//doc[./int[@name='id']='1']",
- "//doc[./int[@name='id']='2']",
- "//doc[./int[@name='id']='3']",
+ "//doc[./str[@name='id']='1']",
+ "//doc[./str[@name='id']='2']",
+ "//doc[./str[@name='id']='3']",
"//result[@numFound='3']"
);
@@ -246,29 +246,29 @@ public class TestComplexPhraseQParserPlu
assertQ(req("q", "{!complexphrase} name:\"protein digest\" AND text:\"dna rules\"")
, "//result[@numFound='1']"
- , "//doc[./int[@name='id']='3']"
+ , "//doc[./str[@name='id']='3']"
);
assertQ(req("q", "{!complexphrase} name:\"prot* dige*\" AND text:\"d* r*\"")
, "//result[@numFound='1']"
- , "//doc[./int[@name='id']='3']"
+ , "//doc[./str[@name='id']='3']"
);
assertQ(req("q", "{!complexphrase inOrder=\"false\"} name:\"dna* rule*\" AND text:\"prot* diges*\"")
, "//result[@numFound='1']"
- , "//doc[./int[@name='id']='1']"
+ , "//doc[./str[@name='id']='1']"
);
- assertQ(req("q", "{!unorderedcomplexphrase} name:\"protein digest\" AND text:\"dna rules\"~2")
+ assertQ(req("q", "{!complexphrase inOrder=false} name:\"protein digest\" AND text:\"dna rules\"~2")
, "//result[@numFound='2']"
- , "//doc[./int[@name='id']='3']"
- , "//doc[./int[@name='id']='4']"
+ , "//doc[./str[@name='id']='3']"
+ , "//doc[./str[@name='id']='4']"
);
- assertQ(req("q", "{!unorderedcomplexphrase inOrder=\"true\"} name:\"protein digest\" AND text:\"dna rules\"")
+ assertQ(req("q", "{!complexphrase inOrder=\"true\"} name:\"protein digest\" AND text:\"dna rules\"")
, "//result[@numFound='1']"
- , "//doc[./int[@name='id']='3']"
+ , "//doc[./str[@name='id']='3']"
);
}
@@ -290,49 +290,49 @@ public class TestComplexPhraseQParserPlu
*/
assertQ(req("q", "{!complexphrase} \"protein digest\"")
, "//result[@numFound='1']"
- , "//doc[./int[@name='id']='1']"
+ , "//doc[./str[@name='id']='1']"
);
assertQ(req("q", "{!complexphrase} \"pro* di*\"")
, "//result[@numFound='1']"
- , "//doc[./int[@name='id']='1']"
+ , "//doc[./str[@name='id']='1']"
);
assertQ(req("q", "{!complexphrase} name:\"protein digest\"")
, "//result[@numFound='1']"
- , "//doc[./int[@name='id']='3']"
+ , "//doc[./str[@name='id']='3']"
);
assertQ(req("q", "{!complexphrase} name:\"pro* di*\"")
, "//result[@numFound='1']"
- , "//doc[./int[@name='id']='3']"
+ , "//doc[./str[@name='id']='3']"
);
/**
* unordered phrase query returns two documents.
*/
- assertQ(req("q", "{!unorderedcomplexphrase} \"digest protein\"")
+ assertQ(req("q", "{!complexphrase inOrder=false} \"digest protein\"")
, "//result[@numFound='2']"
- , "//doc[./int[@name='id']='1']"
- , "//doc[./int[@name='id']='2']"
+ , "//doc[./str[@name='id']='1']"
+ , "//doc[./str[@name='id']='2']"
);
- assertQ(req("q", "{!unorderedcomplexphrase} \"di* pro*\"")
+ assertQ(req("q", "{!complexphrase inOrder=false} \"di* pro*\"")
, "//result[@numFound='2']"
- , "//doc[./int[@name='id']='1']"
- , "//doc[./int[@name='id']='2']"
+ , "//doc[./str[@name='id']='1']"
+ , "//doc[./str[@name='id']='2']"
);
- assertQ(req("q", "{!unorderedcomplexphrase} name:\"digest protein\"")
+ assertQ(req("q", "{!complexphrase inOrder=false} name:\"digest protein\"")
, "//result[@numFound='2']"
- , "//doc[./int[@name='id']='3']"
- , "//doc[./int[@name='id']='4']"
+ , "//doc[./str[@name='id']='3']"
+ , "//doc[./str[@name='id']='4']"
);
- assertQ(req("q", "{!unorderedcomplexphrase} name:\"di* pro*\"")
+ assertQ(req("q", "{!complexphrase inOrder=false} name:\"di* pro*\"")
, "//result[@numFound='2']"
- , "//doc[./int[@name='id']='3']"
- , "//doc[./int[@name='id']='4']"
+ , "//doc[./str[@name='id']='3']"
+ , "//doc[./str[@name='id']='4']"
);
/**
@@ -340,8 +340,13 @@ public class TestComplexPhraseQParserPlu
*/
assertQ(req("q", "{!complexphrase inOrder=false} \"di* pro*\"")
, "//result[@numFound='2']"
- , "//doc[./int[@name='id']='1']"
- , "//doc[./int[@name='id']='2']"
+ , "//doc[./str[@name='id']='1']"
+ , "//doc[./str[@name='id']='2']"
+ );
+
+
+ assertQ(req("q", "{!complexphrase inOrder=true} \"di* pro*\"")
+ , "//result[@numFound='1']"
);
/**
@@ -349,8 +354,8 @@ public class TestComplexPhraseQParserPlu
*/
assertQ(req("q", "{!complexphrase inOrder=false df=name} \"di* pro*\"")
, "//result[@numFound='2']"
- , "//doc[./int[@name='id']='3']"
- , "//doc[./int[@name='id']='4']"
+ , "//doc[./str[@name='id']='3']"
+ , "//doc[./str[@name='id']='4']"
);
}
/**
@@ -369,14 +374,13 @@ public class TestComplexPhraseQParserPlu
assertQ(req("q", "{!complexphrase} \"sulfur-reducing bacteria\"")
, "//result[@numFound='2']"
- , "//doc[./int[@name='id']='1']"
- , "//doc[./int[@name='id']='2']"
+ , "//doc[./str[@name='id']='1']"
+ , "//doc[./str[@name='id']='2']"
);
+ // the analysis for "name" currently does not break on "-" (only whitespace) and thus only matches one doc
assertQ(req("q", "{!complexphrase} name:\"sulfur-reducing bacteria\"")
- , "//result[@numFound='2']"
- , "//doc[./int[@name='id']='3']"
- , "//doc[./int[@name='id']='4']"
+ , "//result[@numFound='1']"
);
}
}