You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mi...@apache.org on 2014/01/28 18:00:13 UTC
svn commit: r1562125 - in /lucene/dev/branches/branch_4x: ./ lucene/
lucene/core/ lucene/core/src/java/org/apache/lucene/search/
lucene/core/src/test/org/apache/lucene/search/ lucene/join/
lucene/join/src/java/org/apache/lucene/search/join/ lucene/join...
Author: mikemccand
Date: Tue Jan 28 17:00:13 2014
New Revision: 1562125
URL: http://svn.apache.org/r1562125
Log:
LUCENE-5409: fix ToParentBlockJoinQuery.rewrite to correctly preserve the origChildQuery after more than one iteration of rewrite
Modified:
lucene/dev/branches/branch_4x/ (props changed)
lucene/dev/branches/branch_4x/lucene/ (props changed)
lucene/dev/branches/branch_4x/lucene/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_4x/lucene/core/ (props changed)
lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java
lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSubScorerFreqs.java
lucene/dev/branches/branch_4x/lucene/join/ (props changed)
lucene/dev/branches/branch_4x/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java
lucene/dev/branches/branch_4x/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java
lucene/dev/branches/branch_4x/lucene/test-framework/ (props changed)
lucene/dev/branches/branch_4x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java
Modified: lucene/dev/branches/branch_4x/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/CHANGES.txt?rev=1562125&r1=1562124&r2=1562125&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_4x/lucene/CHANGES.txt Tue Jan 28 17:00:13 2014
@@ -139,6 +139,10 @@ Bug fixes
* LUCENE-5415: SpanMultiTermQueryWrapper didn't handle its boost in
hashcode/equals/tostring/rewrite. (Robert Muir)
+* LUCENE-5409: ToParentBlockJoinCollector.getTopGroups would fail to
+ return any groups when the joined query required more than one
+ rewrite step (Peng Cheng via Mike McCandless)
+
API Changes
* LUCENE-5339: The facet module was simplified/reworked to make the
Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java?rev=1562125&r1=1562124&r2=1562125&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java Tue Jan 28 17:00:13 2014
@@ -62,8 +62,9 @@ public class BooleanQuery extends Query
* Default value is 1024.
*/
public static void setMaxClauseCount(int maxClauseCount) {
- if (maxClauseCount < 1)
+ if (maxClauseCount < 1) {
throw new IllegalArgumentException("maxClauseCount must be >= 1");
+ }
BooleanQuery.maxClauseCount = maxClauseCount;
}
@@ -138,8 +139,9 @@ public class BooleanQuery extends Query
* @see #getMaxClauseCount()
*/
public void add(BooleanClause clause) {
- if (clauses.size() >= maxClauseCount)
+ if (clauses.size() >= maxClauseCount) {
throw new TooManyClauses();
+ }
clauses.add(clause);
}
@@ -182,7 +184,9 @@ public class BooleanQuery extends Query
BooleanClause c = clauses.get(i);
Weight w = c.getQuery().createWeight(searcher);
weights.add(w);
- if (!c.isProhibited()) maxCoord++;
+ if (!c.isProhibited()) {
+ maxCoord++;
+ }
}
}
@@ -195,9 +199,10 @@ public class BooleanQuery extends Query
for (int i = 0 ; i < weights.size(); i++) {
// call sumOfSquaredWeights for all clauses in case of side effects
float s = weights.get(i).getValueForNormalization(); // sum sub weights
- if (!clauses.get(i).isProhibited())
+ if (!clauses.get(i).isProhibited()) {
// only add to sum for non-prohibited clauses
sum += s;
+ }
}
sum *= getBoost() * getBoost(); // boost each sub-weight
@@ -258,8 +263,9 @@ public class BooleanQuery extends Query
sumExpl.addDetail(r);
fail = true;
}
- if (c.getOccur() == Occur.SHOULD)
+ if (c.getOccur() == Occur.SHOULD) {
shouldMatchCount++;
+ }
} else if (c.isRequired()) {
Explanation r = new Explanation(0.0f, "no match on required clause (" + c.getQuery().toString() + ")");
r.addDetail(e);
@@ -422,8 +428,9 @@ public class BooleanQuery extends Query
}
if (clone != null) {
return clone; // some clauses rewrote
- } else
+ } else {
return this; // no clauses rewrote
+ }
}
// inherit javadoc
@@ -447,17 +454,18 @@ public class BooleanQuery extends Query
@Override
public String toString(String field) {
StringBuilder buffer = new StringBuilder();
- boolean needParens=(getBoost() != 1.0) || (getMinimumNumberShouldMatch()>0) ;
+ boolean needParens= getBoost() != 1.0 || getMinimumNumberShouldMatch() > 0;
if (needParens) {
buffer.append("(");
}
for (int i = 0 ; i < clauses.size(); i++) {
BooleanClause c = clauses.get(i);
- if (c.isProhibited())
+ if (c.isProhibited()) {
buffer.append("-");
- else if (c.isRequired())
+ } else if (c.isRequired()) {
buffer.append("+");
+ }
Query subQuery = c.getQuery();
if (subQuery != null) {
@@ -472,8 +480,9 @@ public class BooleanQuery extends Query
buffer.append("null");
}
- if (i != clauses.size()-1)
+ if (i != clauses.size()-1) {
buffer.append(" ");
+ }
}
if (needParens) {
@@ -485,8 +494,7 @@ public class BooleanQuery extends Query
buffer.append(getMinimumNumberShouldMatch());
}
- if (getBoost() != 1.0f)
- {
+ if (getBoost() != 1.0f) {
buffer.append(ToStringUtils.boost(getBoost()));
}
@@ -496,10 +504,11 @@ public class BooleanQuery extends Query
/** Returns true iff <code>o</code> is equal to this. */
@Override
public boolean equals(Object o) {
- if (!(o instanceof BooleanQuery))
+ if (!(o instanceof BooleanQuery)) {
return false;
+ }
BooleanQuery other = (BooleanQuery)o;
- return (this.getBoost() == other.getBoost())
+ return this.getBoost() == other.getBoost()
&& this.clauses.equals(other.clauses)
&& this.getMinimumNumberShouldMatch() == other.getMinimumNumberShouldMatch()
&& this.disableCoord == other.disableCoord;
Modified: lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSubScorerFreqs.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSubScorerFreqs.java?rev=1562125&r1=1562124&r2=1562125&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSubScorerFreqs.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSubScorerFreqs.java Tue Jan 28 17:00:13 2014
@@ -92,7 +92,7 @@ public class TestSubScorerFreqs extends
public void setSubScorers(Scorer scorer, String relationship) {
for (ChildScorer child : scorer.getChildren()) {
- if (relationships.contains(child.relationship)) {
+ if (scorer instanceof AssertingScorer || relationships.contains(child.relationship)) {
setSubScorers(child.child, child.relationship);
}
}
@@ -180,16 +180,17 @@ public class TestSubScorerFreqs extends
assertEquals(includeOptional ? 5 : 4, doc0.size());
assertEquals(1.0F, doc0.get(aQuery), FLOAT_TOLERANCE);
assertEquals(4.0F, doc0.get(dQuery), FLOAT_TOLERANCE);
- if (includeOptional)
+ if (includeOptional) {
assertEquals(3.0F, doc0.get(cQuery), FLOAT_TOLERANCE);
+ }
Map<Query, Float> doc1 = c.docCounts.get(++i);
assertEquals(includeOptional ? 5 : 4, doc1.size());
assertEquals(1.0F, doc1.get(aQuery), FLOAT_TOLERANCE);
assertEquals(1.0F, doc1.get(dQuery), FLOAT_TOLERANCE);
- if (includeOptional)
+ if (includeOptional) {
assertEquals(1.0F, doc1.get(cQuery), FLOAT_TOLERANCE);
-
+ }
}
}
}
Modified: lucene/dev/branches/branch_4x/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java?rev=1562125&r1=1562124&r2=1562125&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java (original)
+++ lucene/dev/branches/branch_4x/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java Tue Jan 28 17:00:13 2014
@@ -453,7 +453,7 @@ public class ToParentBlockJoinQuery exte
public Query rewrite(IndexReader reader) throws IOException {
final Query childRewrite = childQuery.rewrite(reader);
if (childRewrite != childQuery) {
- Query rewritten = new ToParentBlockJoinQuery(childQuery,
+ Query rewritten = new ToParentBlockJoinQuery(origChildQuery,
childRewrite,
parentsFilter,
scoreMode);
Modified: lucene/dev/branches/branch_4x/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java?rev=1562125&r1=1562124&r2=1562125&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java (original)
+++ lucene/dev/branches/branch_4x/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java Tue Jan 28 17:00:13 2014
@@ -201,7 +201,7 @@ public class TestBlockJoin extends Lucen
childDoc = s.doc(hits.scoreDocs[0].doc);
//System.out.println("CHILD = " + childDoc + " docID=" + hits.scoreDocs[0].doc);
assertEquals("java", childDoc.get("skill"));
- assertEquals(2007, (childDoc.getField("year")).numericValue());
+ assertEquals(2007, childDoc.getField("year").numericValue());
assertEquals("Lisa", getParentDoc(r, parentsFilter, hits.scoreDocs[0].doc).get("name"));
// Test with filter on child docs:
@@ -213,6 +213,54 @@ public class TestBlockJoin extends Lucen
dir.close();
}
+ public void testBugCausedByRewritingTwice() throws IOException {
+ final Directory dir = newDirectory();
+ final RandomIndexWriter w = new RandomIndexWriter(random(), dir);
+
+ final List<Document> docs = new ArrayList<Document>();
+
+ for (int i=0;i<10;i++) {
+ docs.clear();
+ docs.add(makeJob("ruby", i));
+ docs.add(makeJob("java", 2007));
+ docs.add(makeResume("Frank", "United States"));
+ w.addDocuments(docs);
+ }
+
+ IndexReader r = w.getReader();
+ w.close();
+ IndexSearcher s = newSearcher(r);
+
+ MultiTermQuery qc = NumericRangeQuery.newIntRange("year", 2007, 2007, true, true);
+ // Hacky: this causes the query to need 2 rewrite
+ // iterations:
+ qc.setRewriteMethod(MultiTermQuery.CONSTANT_SCORE_BOOLEAN_QUERY_REWRITE);
+
+ Filter parentsFilter = new FixedBitSetCachingWrapperFilter(new QueryWrapperFilter(new TermQuery(new Term("docType", "resume"))));
+
+ int h1 = qc.hashCode();
+ Query qw1 = qc.rewrite(r);
+ int h2 = qw1.hashCode();
+ Query qw2 = qw1.rewrite(r);
+ int h3 = qw2.hashCode();
+
+ assertTrue(h1 != h2);
+ assertTrue(h2 != h3);
+ assertTrue(h3 != h1);
+
+ ToParentBlockJoinQuery qp = new ToParentBlockJoinQuery(qc, parentsFilter, ScoreMode.Max);
+ ToParentBlockJoinCollector c = new ToParentBlockJoinCollector(Sort.RELEVANCE, 10, true, true);
+
+ s.search(qp, c);
+ TopGroups<Integer> groups = c.getTopGroups(qp, Sort.INDEXORDER, 0, 10, 0, true);
+ for (GroupDocs<Integer> group : groups.groups) {
+ assertEquals(1, group.totalHits);
+ }
+
+ r.close();
+ dir.close();
+ }
+
protected QueryWrapperFilter skill(String skill) {
return new QueryWrapperFilter(new TermQuery(new Term("skill", skill)));
}
Modified: lucene/dev/branches/branch_4x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java?rev=1562125&r1=1562124&r2=1562125&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java (original)
+++ lucene/dev/branches/branch_4x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java Tue Jan 28 17:00:13 2014
@@ -151,7 +151,11 @@ public class AssertingScorer extends Sco
@Override
public Collection<ChildScorer> getChildren() {
- return in.getChildren();
+ // We cannot hide that we hold a single child, else
+ // collectors (e.g. ToParentBlockJoinCollector) that
+ // need to walk the scorer tree will miss/skip the
+ // Scorer we wrap:
+ return Collections.singletonList(new ChildScorer(in, "SHOULD"));
}
@Override