From 6d1353bc590bf31f4b5c5942935c316c87cdc19e Mon Sep 17 00:00:00 2001 From: "Chris M. Hostetter" Date: Fri, 25 Feb 2011 00:15:42 +0000 Subject: [PATCH] LUCENE-2936: PhraseQuery score explanations were not correctly identifying matches vs non-matches git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1074357 13f79535-47bb-0310-9956-ffa450edef68 --- lucene/CHANGES.txt | 3 + .../org/apache/lucene/search/PhraseQuery.java | 7 +-- .../org/apache/lucene/search/CheckHits.java | 13 +++-- .../lucene/search/TestExplanations.java | 8 ++- .../lucene/search/TestSimpleExplanations.java | 58 +++++++++++++++++++ 5 files changed, 78 insertions(+), 11 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 2835d89937c..67e9246b88c 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -355,6 +355,9 @@ Bug fixes with more document deletions is requested before a reader with fewer deletions, provided they share some segments. (yonik) +* LUCENE-2936: PhraseQuery score explanations were not correctly + identifying matches vs non-matches. (hossman) + ======================= Lucene 3.x (not yet released) ======================= Changes in backwards compatibility policy diff --git a/lucene/src/java/org/apache/lucene/search/PhraseQuery.java b/lucene/src/java/org/apache/lucene/search/PhraseQuery.java index 8c71ad78bd5..2c8d977fa82 100644 --- a/lucene/src/java/org/apache/lucene/search/PhraseQuery.java +++ b/lucene/src/java/org/apache/lucene/search/PhraseQuery.java @@ -224,7 +224,7 @@ public class PhraseQuery extends Query { public Explanation explain(AtomicReaderContext context, int doc) throws IOException { - Explanation result = new Explanation(); + ComplexExplanation result = new ComplexExplanation(); result.setDescription("weight("+getQuery()+" in "+doc+"), product of:"); StringBuilder docFreqs = new StringBuilder(); @@ -303,10 +303,7 @@ public class PhraseQuery extends Query { // combine them result.setValue(queryExpl.getValue() * fieldExpl.getValue()); - - if (queryExpl.getValue() == 1.0f) - return fieldExpl; - + result.setMatch(tfExplanation.isMatch()); return result; } } diff --git a/lucene/src/test-framework/org/apache/lucene/search/CheckHits.java b/lucene/src/test-framework/org/apache/lucene/search/CheckHits.java index 6846e59deca..fd6a8f8afd0 100644 --- a/lucene/src/test-framework/org/apache/lucene/search/CheckHits.java +++ b/lucene/src/test-framework/org/apache/lucene/search/CheckHits.java @@ -39,8 +39,8 @@ public class CheckHits { /** * Tests that all documents up to maxDoc which are *not* in the - * expected result set, have an explanation which indicates no match - * (ie: Explanation value of 0.0f) + * expected result set, have an explanation which indicates that + * the document does not match */ public static void checkNoMatchExplanations(Query q, String defaultFieldName, IndexSearcher searcher, int[] results) @@ -59,9 +59,9 @@ public class CheckHits { Explanation exp = searcher.explain(q, doc); Assert.assertNotNull("Explanation of [["+d+"]] for #"+doc+" is null", exp); - Assert.assertEquals("Explanation of [["+d+"]] for #"+doc+ - " doesn't indicate non-match: " + exp.toString(), - 0.0f, exp.getValue(), 0.0f); + Assert.assertFalse("Explanation of [["+d+"]] for #"+doc+ + " doesn't indicate non-match: " + exp.toString(), + exp.isMatch()); } } @@ -484,6 +484,9 @@ public class CheckHits { Assert.assertNotNull("Explanation of [["+d+"]] for #"+doc+" is null", exp); verifyExplanation(d,doc,scorer.score(),deep,exp); + Assert.assertTrue("Explanation of [["+d+"]] for #"+ doc + + " does not indicate match: " + exp.toString(), + exp.isMatch()); } @Override public void setNextReader(AtomicReaderContext context) { diff --git a/lucene/src/test/org/apache/lucene/search/TestExplanations.java b/lucene/src/test/org/apache/lucene/search/TestExplanations.java index 3f2712af511..467c9477484 100644 --- a/lucene/src/test/org/apache/lucene/search/TestExplanations.java +++ b/lucene/src/test/org/apache/lucene/search/TestExplanations.java @@ -52,7 +52,10 @@ public class TestExplanations extends LuceneTestCase { protected Directory directory; public static final String KEY = "KEY"; + // boost on this field is the same as the iterator for the doc public static final String FIELD = "field"; + // same contents, but no field boost + public static final String ALTFIELD = "alt"; public static final QueryParser qp = new QueryParser(TEST_VERSION_CURRENT, FIELD, new MockAnalyzer()); @@ -72,7 +75,10 @@ public class TestExplanations extends LuceneTestCase { for (int i = 0; i < docFields.length; i++) { Document doc = new Document(); doc.add(newField(KEY, ""+i, Field.Store.NO, Field.Index.NOT_ANALYZED)); - doc.add(newField(FIELD, docFields[i], Field.Store.NO, Field.Index.ANALYZED)); + Field f = newField(FIELD, docFields[i], Field.Store.NO, Field.Index.ANALYZED); + f.setBoost(i); + doc.add(f); + doc.add(newField(ALTFIELD, docFields[i], Field.Store.NO, Field.Index.ANALYZED)); writer.addDocument(doc); } reader = writer.getReader(); diff --git a/lucene/src/test/org/apache/lucene/search/TestSimpleExplanations.java b/lucene/src/test/org/apache/lucene/search/TestSimpleExplanations.java index 116b10a6e20..de21d8aaf2a 100644 --- a/lucene/src/test/org/apache/lucene/search/TestSimpleExplanations.java +++ b/lucene/src/test/org/apache/lucene/search/TestSimpleExplanations.java @@ -289,4 +289,62 @@ public class TestSimpleExplanations extends TestExplanations { qtest(q, new int[] { 0,3 }); } + + /* BQ of TQ: using alt so some fields have zero boost and some don't */ + + public void testMultiFieldBQ1() throws Exception { + qtest("+w1 +alt:w2", new int[] { 0,1,2,3 }); + } + public void testMultiFieldBQ2() throws Exception { + qtest("+yy +alt:w3", new int[] { 2,3 }); + } + public void testMultiFieldBQ3() throws Exception { + qtest("yy +alt:w3", new int[] { 0,1,2,3 }); + } + public void testMultiFieldBQ4() throws Exception { + qtest("w1 (-xx alt:w2)", new int[] { 0,1,2,3 }); + } + public void testMultiFieldBQ5() throws Exception { + qtest("w1 (+alt:qq alt:w2)", new int[] { 0,1,2,3 }); + } + public void testMultiFieldBQ6() throws Exception { + qtest("w1 -(-alt:qq alt:w5)", new int[] { 1,2,3 }); + } + public void testMultiFieldBQ7() throws Exception { + qtest("+w1 +(alt:qq (alt:xx -alt:w2) (+alt:w3 +alt:w4))", new int[] { 0 }); + } + public void testMultiFieldBQ8() throws Exception { + qtest("+alt:w1 (qq (alt:xx -w2) (+alt:w3 +w4))", new int[] { 0,1,2,3 }); + } + public void testMultiFieldBQ9() throws Exception { + qtest("+w1 (alt:qq (-xx w2) -(+alt:w3 +w4))", new int[] { 0,1,2,3 }); + } + public void testMultiFieldBQ10() throws Exception { + qtest("+w1 +(alt:qq (-xx alt:w2) -(+alt:w3 +w4))", new int[] { 1 }); + } + + /* BQ of PQ: using alt so some fields have zero boost and some don't */ + + public void testMultiFieldBQofPQ1() throws Exception { + qtest("\"w1 w2\" alt:\"w1 w2\"", new int[] { 0 }); + } + public void testMultiFieldBQofPQ2() throws Exception { + qtest("\"w1 w3\" alt:\"w1 w3\"", new int[] { 1,3 }); + } + public void testMultiFieldBQofPQ3() throws Exception { + qtest("\"w1 w2\"~1 alt:\"w1 w2\"~1", new int[] { 0,1,2 }); + } + public void testMultiFieldBQofPQ4() throws Exception { + qtest("\"w2 w3\"~1 alt:\"w2 w3\"~1", new int[] { 0,1,2,3 }); + } + public void testMultiFieldBQofPQ5() throws Exception { + qtest("\"w3 w2\"~1 alt:\"w3 w2\"~1", new int[] { 1,3 }); + } + public void testMultiFieldBQofPQ6() throws Exception { + qtest("\"w3 w2\"~2 alt:\"w3 w2\"~2", new int[] { 0,1,3 }); + } + public void testMultiFieldBQofPQ7() throws Exception { + qtest("\"w3 w2\"~3 alt:\"w3 w2\"~3", new int[] { 0,1,2,3 }); + } + }