From b4d27a0dec29265985e688106e4acf7ed20e5b9e Mon Sep 17 00:00:00 2001 From: Marcus Date: Wed, 26 Apr 2023 21:42:43 -0400 Subject: [PATCH 01/21] initial commit of score mode explain. --- .../search/join/ToParentBlockJoinQuery.java | 50 +++++++++++----- .../search/join/TestBlockJoinScorer.java | 57 ++++++++++++++++--- 2 files changed, 86 insertions(+), 21 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index 1e4fe34a3653..00077b09fbaa 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -170,11 +170,11 @@ public long cost() { @Override public Explanation explain(LeafReaderContext context, int doc) throws IOException { - BlockJoinScorer scorer = (BlockJoinScorer) scorer(context); - if (scorer != null && scorer.iterator().advance(doc) == doc) { - return scorer.explain(context, in); - } - return Explanation.noMatch("Not a match"); + BlockJoinScorer scorer = (BlockJoinScorer) scorer(context); + if (scorer != null && scorer.iterator().advance(doc) == doc) { + return scorer.explain(context, in, scoreMode); + } + return Explanation.noMatch("Not a match"); } @Override @@ -391,35 +391,57 @@ private void setScoreAndFreq() throws IOException { } this.score = (float) score; } - - public Explanation explain(LeafReaderContext context, Weight childWeight) throws IOException { + /* + * This instance of Explanation requires three parameters, context, childWeight, and scoreMode. + * The scoreMode parameter considers Avg, Total, Min, Max, and None. + * */ + public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreMode scoreMode) throws IOException { int prevParentDoc = parentBits.prevSetBit(parentApproximation.docID() - 1); int start = context.docBase + prevParentDoc + 1; // +1 b/c prevParentDoc is previous parent doc int end = context.docBase + parentApproximation.docID() - 1; // -1 b/c parentDoc is parent doc - Explanation bestChild = null; + float aggregatedScore; + aggregatedScore = 0; int matches = 0; for (int childDoc = start; childDoc <= end; childDoc++) { Explanation child = childWeight.explain(context, childDoc - context.docBase); if (child.isMatch()) { matches++; - if (bestChild == null - || child.getValue().floatValue() > bestChild.getValue().floatValue()) { - bestChild = child; + float childScore = child.getValue().floatValue(); + switch (scoreMode) { + case None: + break; + case Avg: + case Total: + aggregatedScore += childScore; + break; + case Max: + aggregatedScore = Math.max(aggregatedScore, childScore); + break; + case Min: + if (matches == 1 || childScore < aggregatedScore) { + aggregatedScore = childScore; + } + break; } } } + // Calculate the average if scoreMode is Avg + if (scoreMode == ScoreMode.Avg && matches > 0) { + aggregatedScore /= matches; + } + return Explanation.match( score(), String.format( Locale.ROOT, - "Score based on %d child docs in range from %d to %d, best match:", + "Score based on %d child docs in range from %d to %d, using score mode %s:", matches, start, - end), - bestChild); + end, + scoreMode.toString())); } } diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java index 22f4ae6a5d7c..f5d018369068 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java @@ -24,14 +24,8 @@ import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.Term; -import org.apache.lucene.search.DocIdSetIterator; -import org.apache.lucene.search.IndexSearcher; -import org.apache.lucene.search.MatchAllDocsQuery; -import org.apache.lucene.search.Query; +import org.apache.lucene.search.*; import org.apache.lucene.search.ScoreMode; -import org.apache.lucene.search.Scorer; -import org.apache.lucene.search.TermQuery; -import org.apache.lucene.search.Weight; import org.apache.lucene.store.Directory; import org.apache.lucene.tests.index.RandomIndexWriter; import org.apache.lucene.tests.util.LuceneTestCase; @@ -108,6 +102,55 @@ public void testScoreNone() throws IOException { scorer.setMinCompetitiveScore(Math.nextUp(0f)); assertEquals(DocIdSetIterator.NO_MORE_DOCS, scorer.iterator().nextDoc()); + reader.close(); + dir.close(); + } + public void testExplainScoreModeSum() throws IOException { + Directory dir = newDirectory(); + RandomIndexWriter w = + new RandomIndexWriter( + random(), + dir, + newIndexWriterConfig() + .setMergePolicy( + newLogMergePolicy(random().nextBoolean()))); + List docs = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + docs.clear(); + for (int j = 0; j < i; j++) { + Document child = new Document(); + child.add(newStringField("value", Integer.toString(j), Field.Store.YES)); + child.add(newStringField("docType", "child", Field.Store.NO)); + docs.add(child); + } + Document parent = new Document(); + parent.add(newStringField("value", Integer.toString(i), Field.Store.YES)); + parent.add(newStringField("docType", "parent", Field.Store.NO)); + docs.add(parent); + w.addDocuments(docs); + } + w.forceMerge(1); + + IndexReader reader = w.getReader(); + w.close(); + IndexSearcher searcher = newSearcher(reader); + + BitSetProducer parentsFilter = + new QueryBitSetProducer(new TermQuery(new Term("docType", "parent"))); + CheckJoinIndex.check(reader, parentsFilter); + + Query childQuery = new TermQuery(new Term("docType", "child")); + ToParentBlockJoinQuery query = + new ToParentBlockJoinQuery( + childQuery, parentsFilter, org.apache.lucene.search.join.ScoreMode.Total); + + + TopDocs topDocs = searcher.search(query, 10); + for (ScoreDoc scoreDoc : topDocs.scoreDocs) { + Explanation explanation = searcher.explain(query, scoreDoc.doc); + assertEquals(scoreDoc.score, explanation.getValue().floatValue(), 0.001f); + } + reader.close(); dir.close(); } From 6f797dc4c672894de563d0f687b4b419530847c6 Mon Sep 17 00:00:00 2001 From: Marcus Date: Wed, 26 Apr 2023 21:45:14 -0400 Subject: [PATCH 02/21] tidy up. --- .../search/join/ToParentBlockJoinQuery.java | 21 ++++++++++--------- .../search/join/TestBlockJoinScorer.java | 18 +++++++--------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index 00077b09fbaa..b8920879c20d 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -170,11 +170,11 @@ public long cost() { @Override public Explanation explain(LeafReaderContext context, int doc) throws IOException { - BlockJoinScorer scorer = (BlockJoinScorer) scorer(context); - if (scorer != null && scorer.iterator().advance(doc) == doc) { - return scorer.explain(context, in, scoreMode); - } - return Explanation.noMatch("Not a match"); + BlockJoinScorer scorer = (BlockJoinScorer) scorer(context); + if (scorer != null && scorer.iterator().advance(doc) == doc) { + return scorer.explain(context, in, scoreMode); + } + return Explanation.noMatch("Not a match"); } @Override @@ -392,10 +392,11 @@ private void setScoreAndFreq() throws IOException { this.score = (float) score; } /* - * This instance of Explanation requires three parameters, context, childWeight, and scoreMode. - * The scoreMode parameter considers Avg, Total, Min, Max, and None. - * */ - public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreMode scoreMode) throws IOException { + * This instance of Explanation requires three parameters, context, childWeight, and scoreMode. + * The scoreMode parameter considers Avg, Total, Min, Max, and None. + * */ + public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreMode scoreMode) + throws IOException { int prevParentDoc = parentBits.prevSetBit(parentApproximation.docID() - 1); int start = context.docBase + prevParentDoc + 1; // +1 b/c prevParentDoc is previous parent doc @@ -441,7 +442,7 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM matches, start, end, - scoreMode.toString())); + scoreMode.toString())); } } diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java index f5d018369068..792362adbe9a 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java @@ -105,15 +105,14 @@ public void testScoreNone() throws IOException { reader.close(); dir.close(); } + public void testExplainScoreModeSum() throws IOException { Directory dir = newDirectory(); RandomIndexWriter w = - new RandomIndexWriter( - random(), - dir, - newIndexWriterConfig() - .setMergePolicy( - newLogMergePolicy(random().nextBoolean()))); + new RandomIndexWriter( + random(), + dir, + newIndexWriterConfig().setMergePolicy(newLogMergePolicy(random().nextBoolean()))); List docs = new ArrayList<>(); for (int i = 0; i < 10; i++) { docs.clear(); @@ -136,14 +135,13 @@ public void testExplainScoreModeSum() throws IOException { IndexSearcher searcher = newSearcher(reader); BitSetProducer parentsFilter = - new QueryBitSetProducer(new TermQuery(new Term("docType", "parent"))); + new QueryBitSetProducer(new TermQuery(new Term("docType", "parent"))); CheckJoinIndex.check(reader, parentsFilter); Query childQuery = new TermQuery(new Term("docType", "child")); ToParentBlockJoinQuery query = - new ToParentBlockJoinQuery( - childQuery, parentsFilter, org.apache.lucene.search.join.ScoreMode.Total); - + new ToParentBlockJoinQuery( + childQuery, parentsFilter, org.apache.lucene.search.join.ScoreMode.Total); TopDocs topDocs = searcher.search(query, 10); for (ScoreDoc scoreDoc : topDocs.scoreDocs) { From 2b5e08454372b836aa7038d9546fe93cf900b7d6 Mon Sep 17 00:00:00 2001 From: Marcus Date: Wed, 26 Apr 2023 21:57:01 -0400 Subject: [PATCH 03/21] tidy a version file that was unmodified. --- lucene/core/src/java/org/apache/lucene/util/Version.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/util/Version.java b/lucene/core/src/java/org/apache/lucene/util/Version.java index 3016ee0ccf9b..f997ff93d602 100644 --- a/lucene/core/src/java/org/apache/lucene/util/Version.java +++ b/lucene/core/src/java/org/apache/lucene/util/Version.java @@ -100,8 +100,7 @@ public final class Version { * * @deprecated Use latest */ - @Deprecated - public static final Version LUCENE_9_7_0 = new Version(9, 7, 0); + @Deprecated public static final Version LUCENE_9_7_0 = new Version(9, 7, 0); /** * Match settings and bugs in Lucene's 10.0.0 release. From dcedc7a33c20456d89d998e9dd5eecbc7d32ee08 Mon Sep 17 00:00:00 2001 From: Marcus Date: Wed, 26 Apr 2023 22:13:11 -0400 Subject: [PATCH 04/21] moving to testBlockJoin. --- .../search/join/TestBlockJoinScorer.java | 55 +++---------------- 1 file changed, 7 insertions(+), 48 deletions(-) diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java index 792362adbe9a..22f4ae6a5d7c 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java @@ -24,8 +24,14 @@ import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.Term; -import org.apache.lucene.search.*; +import org.apache.lucene.search.DocIdSetIterator; +import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.search.Query; import org.apache.lucene.search.ScoreMode; +import org.apache.lucene.search.Scorer; +import org.apache.lucene.search.TermQuery; +import org.apache.lucene.search.Weight; import org.apache.lucene.store.Directory; import org.apache.lucene.tests.index.RandomIndexWriter; import org.apache.lucene.tests.util.LuceneTestCase; @@ -105,51 +111,4 @@ public void testScoreNone() throws IOException { reader.close(); dir.close(); } - - public void testExplainScoreModeSum() throws IOException { - Directory dir = newDirectory(); - RandomIndexWriter w = - new RandomIndexWriter( - random(), - dir, - newIndexWriterConfig().setMergePolicy(newLogMergePolicy(random().nextBoolean()))); - List docs = new ArrayList<>(); - for (int i = 0; i < 10; i++) { - docs.clear(); - for (int j = 0; j < i; j++) { - Document child = new Document(); - child.add(newStringField("value", Integer.toString(j), Field.Store.YES)); - child.add(newStringField("docType", "child", Field.Store.NO)); - docs.add(child); - } - Document parent = new Document(); - parent.add(newStringField("value", Integer.toString(i), Field.Store.YES)); - parent.add(newStringField("docType", "parent", Field.Store.NO)); - docs.add(parent); - w.addDocuments(docs); - } - w.forceMerge(1); - - IndexReader reader = w.getReader(); - w.close(); - IndexSearcher searcher = newSearcher(reader); - - BitSetProducer parentsFilter = - new QueryBitSetProducer(new TermQuery(new Term("docType", "parent"))); - CheckJoinIndex.check(reader, parentsFilter); - - Query childQuery = new TermQuery(new Term("docType", "child")); - ToParentBlockJoinQuery query = - new ToParentBlockJoinQuery( - childQuery, parentsFilter, org.apache.lucene.search.join.ScoreMode.Total); - - TopDocs topDocs = searcher.search(query, 10); - for (ScoreDoc scoreDoc : topDocs.scoreDocs) { - Explanation explanation = searcher.explain(query, scoreDoc.doc); - assertEquals(scoreDoc.score, explanation.getValue().floatValue(), 0.001f); - } - - reader.close(); - dir.close(); - } } From 415fd86f19f2f280f8555e8174f39f07c579438f Mon Sep 17 00:00:00 2001 From: Marcus Date: Wed, 26 Apr 2023 22:26:59 -0400 Subject: [PATCH 05/21] ensure score mode is included in match text. --- .../src/test/org/apache/lucene/search/join/TestBlockJoin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java index b77e634aac20..f708671b7b37 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java @@ -896,7 +896,7 @@ public void testRandom() throws Exception { assertEquals(hit.score, explanation.getValue().doubleValue(), 0.0f); Matcher m = Pattern.compile( - "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), best match:") + "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), using score mode %s:") .matcher(explanation.getDescription()); assertTrue("Block Join description not matches", m.matches()); assertTrue("Matched children not positive", Integer.parseInt(m.group(1)) > 0); From daa584c71a097f6d41b57944db756cf99495706f Mon Sep 17 00:00:00 2001 From: Marcus Date: Thu, 27 Apr 2023 03:58:04 -0400 Subject: [PATCH 06/21] I think this is closer to the right track. tests won't run on my rig. --- .../search/join/ToParentBlockJoinQuery.java | 18 +++++++++++------- .../lucene/search/join/TestBlockJoin.java | 3 +-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index b8920879c20d..0c53393337fc 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -405,6 +405,7 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM float aggregatedScore; aggregatedScore = 0; int matches = 0; + Explanation aggregateExplanation = null; for (int childDoc = start; childDoc <= end; childDoc++) { Explanation child = childWeight.explain(context, childDoc - context.docBase); if (child.isMatch()) { @@ -416,33 +417,36 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM case Avg: case Total: aggregatedScore += childScore; + aggregateExplanation = Explanation.match(aggregatedScore, "Aggregated score", aggregateExplanation, child); break; case Max: - aggregatedScore = Math.max(aggregatedScore, childScore); - break; + if (aggregateExplanation == null || childScore > aggregateExplanation.getValue().floatValue()) { + aggregatedScore = childScore; + aggregateExplanation = Explanation.match(aggregatedScore, "Aggregated score", aggregateExplanation, child); + } case Min: if (matches == 1 || childScore < aggregatedScore) { aggregatedScore = childScore; + aggregateExplanation = Explanation.match(aggregatedScore, "Aggregated score", aggregateExplanation, child); } break; } } } - // Calculate the average if scoreMode is Avg if (scoreMode == ScoreMode.Avg && matches > 0) { aggregatedScore /= matches; + aggregateExplanation = Explanation.match(aggregatedScore, "Aggregated score", aggregateExplanation); } - return Explanation.match( score(), String.format( Locale.ROOT, - "Score based on %d child docs in range from %d to %d, using score mode %s:", + "Score based on %d child docs in range from %d to %d, using score mode :%s, best match:", matches, start, - end, - scoreMode.toString())); + end, scoreMode.toString()), + aggregateExplanation); } } diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java index f708671b7b37..237f343f4d84 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java @@ -277,7 +277,6 @@ public void testSimple() throws Exception { CheckHits.checkHitCollector(random(), fullQuery.build(), "country", s, new int[] {2}); TopDocs topDocs = s.search(fullQuery.build(), 1); - // assertEquals(1, results.totalHitCount); assertEquals(1, topDocs.totalHits.value); Document parentDoc = s.storedFields().document(topDocs.scoreDocs[0].doc); @@ -896,7 +895,7 @@ public void testRandom() throws Exception { assertEquals(hit.score, explanation.getValue().doubleValue(), 0.0f); Matcher m = Pattern.compile( - "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), using score mode %s:") + "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), using score mode (None|Avg|Min|Max|Total)") .matcher(explanation.getDescription()); assertTrue("Block Join description not matches", m.matches()); assertTrue("Matched children not positive", Integer.parseInt(m.group(1)) > 0); From 28753664c9310e42230f2fc30eeaa4e3f05c9bb3 Mon Sep 17 00:00:00 2001 From: Marcus Date: Thu, 27 Apr 2023 03:59:07 -0400 Subject: [PATCH 07/21] tidy. --- .../search/join/ToParentBlockJoinQuery.java | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index 0c53393337fc..f05c453624f8 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -417,17 +417,24 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM case Avg: case Total: aggregatedScore += childScore; - aggregateExplanation = Explanation.match(aggregatedScore, "Aggregated score", aggregateExplanation, child); + aggregateExplanation = + Explanation.match( + aggregatedScore, "Aggregated score", aggregateExplanation, child); break; case Max: - if (aggregateExplanation == null || childScore > aggregateExplanation.getValue().floatValue()) { + if (aggregateExplanation == null + || childScore > aggregateExplanation.getValue().floatValue()) { aggregatedScore = childScore; - aggregateExplanation = Explanation.match(aggregatedScore, "Aggregated score", aggregateExplanation, child); + aggregateExplanation = + Explanation.match( + aggregatedScore, "Aggregated score", aggregateExplanation, child); } case Min: if (matches == 1 || childScore < aggregatedScore) { aggregatedScore = childScore; - aggregateExplanation = Explanation.match(aggregatedScore, "Aggregated score", aggregateExplanation, child); + aggregateExplanation = + Explanation.match( + aggregatedScore, "Aggregated score", aggregateExplanation, child); } break; } @@ -436,7 +443,8 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM // Calculate the average if scoreMode is Avg if (scoreMode == ScoreMode.Avg && matches > 0) { aggregatedScore /= matches; - aggregateExplanation = Explanation.match(aggregatedScore, "Aggregated score", aggregateExplanation); + aggregateExplanation = + Explanation.match(aggregatedScore, "Aggregated score", aggregateExplanation); } return Explanation.match( score(), @@ -445,8 +453,9 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM "Score based on %d child docs in range from %d to %d, using score mode :%s, best match:", matches, start, - end, scoreMode.toString()), - aggregateExplanation); + end, + scoreMode.toString()), + aggregateExplanation); } } From cb963359c042a7b3e8db24845b7056280b7a078b Mon Sep 17 00:00:00 2001 From: Marcus Date: Thu, 27 Apr 2023 04:16:09 -0400 Subject: [PATCH 08/21] introduce a variable and string template, simplify return. --- .../search/join/ToParentBlockJoinQuery.java | 30 +++++-------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index f05c453624f8..aa7f2f35e86b 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -21,7 +21,6 @@ import java.io.IOException; import java.util.Collection; import java.util.Collections; -import java.util.Locale; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.ConstantScoreQuery; @@ -411,30 +410,28 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM if (child.isMatch()) { matches++; float childScore = child.getValue().floatValue(); + String scoreDescription = + String.format( + "Score based on %d child docs in range from %d to %d, using score mode :%s, best match:", + matches, start, end, scoreMode.toString()); switch (scoreMode) { case None: break; case Avg: case Total: aggregatedScore += childScore; - aggregateExplanation = - Explanation.match( - aggregatedScore, "Aggregated score", aggregateExplanation, child); + aggregateExplanation = Explanation.match(aggregatedScore, scoreDescription, child); break; case Max: if (aggregateExplanation == null || childScore > aggregateExplanation.getValue().floatValue()) { aggregatedScore = childScore; - aggregateExplanation = - Explanation.match( - aggregatedScore, "Aggregated score", aggregateExplanation, child); + aggregateExplanation = Explanation.match(aggregatedScore, scoreDescription, child); } case Min: if (matches == 1 || childScore < aggregatedScore) { aggregatedScore = childScore; - aggregateExplanation = - Explanation.match( - aggregatedScore, "Aggregated score", aggregateExplanation, child); + aggregateExplanation = Explanation.match(aggregatedScore, scoreDescription, child); } break; } @@ -443,19 +440,8 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM // Calculate the average if scoreMode is Avg if (scoreMode == ScoreMode.Avg && matches > 0) { aggregatedScore /= matches; - aggregateExplanation = - Explanation.match(aggregatedScore, "Aggregated score", aggregateExplanation); } - return Explanation.match( - score(), - String.format( - Locale.ROOT, - "Score based on %d child docs in range from %d to %d, using score mode :%s, best match:", - matches, - start, - end, - scoreMode.toString()), - aggregateExplanation); + return aggregateExplanation; } } From 40ca8548f7faf52c095088ab4f1ca3073d56bc01 Mon Sep 17 00:00:00 2001 From: Marcus Date: Thu, 27 Apr 2023 04:20:50 -0400 Subject: [PATCH 09/21] oops fall through error. --- .../org/apache/lucene/search/join/ToParentBlockJoinQuery.java | 1 + 1 file changed, 1 insertion(+) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index aa7f2f35e86b..60484e06f7e7 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -428,6 +428,7 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM aggregatedScore = childScore; aggregateExplanation = Explanation.match(aggregatedScore, scoreDescription, child); } + break; case Min: if (matches == 1 || childScore < aggregatedScore) { aggregatedScore = childScore; From 9e590a0d06df29ac0f84053fef8f990107468341 Mon Sep 17 00:00:00 2001 From: Marcus Date: Thu, 27 Apr 2023 04:32:55 -0400 Subject: [PATCH 10/21] final failure is comparison between expected value and actual value I need to do more debugging.. --- .../org/apache/lucene/search/join/ToParentBlockJoinQuery.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index 60484e06f7e7..8b6cf4a2997d 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -412,7 +412,7 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM float childScore = child.getValue().floatValue(); String scoreDescription = String.format( - "Score based on %d child docs in range from %d to %d, using score mode :%s, best match:", + "Score based on %d child docs in range from %d to %d, using score mode :%s", matches, start, end, scoreMode.toString()); switch (scoreMode) { case None: From 41e260089d5604fb86b97032270b43f324d5f0a4 Mon Sep 17 00:00:00 2001 From: Marcus Date: Fri, 28 Apr 2023 08:50:24 -0400 Subject: [PATCH 11/21] change data type to match tests, decouple score modes. --- .../search/join/ToParentBlockJoinQuery.java | 81 +++++++++++-------- 1 file changed, 48 insertions(+), 33 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index 8b6cf4a2997d..9b5f3664922b 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.Collection; import java.util.Collections; +import java.util.Locale; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.ConstantScoreQuery; @@ -401,48 +402,62 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM context.docBase + prevParentDoc + 1; // +1 b/c prevParentDoc is previous parent doc int end = context.docBase + parentApproximation.docID() - 1; // -1 b/c parentDoc is parent doc - float aggregatedScore; - aggregatedScore = 0; + Explanation bestChild = null; + double childScoreSum = 0; int matches = 0; - Explanation aggregateExplanation = null; for (int childDoc = start; childDoc <= end; childDoc++) { Explanation child = childWeight.explain(context, childDoc - context.docBase); if (child.isMatch()) { matches++; - float childScore = child.getValue().floatValue(); - String scoreDescription = - String.format( - "Score based on %d child docs in range from %d to %d, using score mode :%s", - matches, start, end, scoreMode.toString()); - switch (scoreMode) { - case None: - break; - case Avg: - case Total: - aggregatedScore += childScore; - aggregateExplanation = Explanation.match(aggregatedScore, scoreDescription, child); - break; - case Max: - if (aggregateExplanation == null - || childScore > aggregateExplanation.getValue().floatValue()) { - aggregatedScore = childScore; - aggregateExplanation = Explanation.match(aggregatedScore, scoreDescription, child); - } - break; - case Min: - if (matches == 1 || childScore < aggregatedScore) { - aggregatedScore = childScore; - aggregateExplanation = Explanation.match(aggregatedScore, scoreDescription, child); - } - break; + childScoreSum += child.getValue().doubleValue(); + + if (scoreMode == ScoreMode.Total) { + continue; + } + + if (bestChild == null + || child.getValue().doubleValue() > bestChild.getValue().doubleValue()) { + bestChild = child; } } } - // Calculate the average if scoreMode is Avg - if (scoreMode == ScoreMode.Avg && matches > 0) { - aggregatedScore /= matches; + + if (bestChild == null) { + return Explanation.match( + score(), + String.format( + Locale.ROOT, + "Score based on 0 child docs in range from %d to %d, best match according to %s:", + matches, + start, + end, + scoreMode)); + } + + if (scoreMode == ScoreMode.Avg) { + double avgScore = matches > 0 ? childScoreSum / (double) matches : 0; + return Explanation.match( + avgScore, + String.format( + Locale.ROOT, + "Score based on %d child docs in range from %d to %d, best match according to %s:", + matches, + start, + end, + scoreMode), + bestChild); + } else { + return Explanation.match( + score(), + String.format( + Locale.ROOT, + "Score based on %d child docs in range from %d to %d, best match according to %s:", + matches, + start, + end, + scoreMode), + bestChild); } - return aggregateExplanation; } } From 23c828068f395ccddae6535657fabe5c1065664a Mon Sep 17 00:00:00 2001 From: Marcus Date: Mon, 1 May 2023 00:02:29 -0400 Subject: [PATCH 12/21] all issues resolved except one. --- .../search/join/ToParentBlockJoinQuery.java | 39 ++++++++++++------- .../lucene/search/join/TestBlockJoin.java | 3 +- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index 9b5f3664922b..588748e611af 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -421,26 +421,39 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM } } } - + // this one causes issues + // if (bestChild == null) { - return Explanation.match( - score(), - String.format( - Locale.ROOT, - "Score based on 0 child docs in range from %d to %d, best match according to %s:", - matches, - start, - end, - scoreMode)); + if (scoreMode == ScoreMode.None) { + double score = bestChild.getValue().doubleValue(); + return Explanation.match( + score, + String.format( + Locale.ROOT, + "Score based on %d child docs in range from %d to %d, using score mode %s", + matches, + start, + end, + scoreMode), + bestChild); + } else { + return Explanation.match( + 0.0f, + String.format( + Locale.ROOT, + "Score based on 0 child docs in range from %d to %d, using score mode %s", + start, + end, + scoreMode)); + } } - if (scoreMode == ScoreMode.Avg) { double avgScore = matches > 0 ? childScoreSum / (double) matches : 0; return Explanation.match( avgScore, String.format( Locale.ROOT, - "Score based on %d child docs in range from %d to %d, best match according to %s:", + "Score based on %d child docs in range from %d to %d, using score mode %s", matches, start, end, @@ -451,7 +464,7 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM score(), String.format( Locale.ROOT, - "Score based on %d child docs in range from %d to %d, best match according to %s:", + "Score based on %d child docs in range from %d to %d, using score mode %s", matches, start, end, diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java index 237f343f4d84..03a384cc667d 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java @@ -892,7 +892,8 @@ public void testRandom() throws Exception { // System.out.println(" hit docID=" + hit.doc + " childId=" + childId + " parentId=" + // document.get("parentID")); assertTrue(explanation.isMatch()); - assertEquals(hit.score, explanation.getValue().doubleValue(), 0.0f); + // This test is failing in strange ways. + // assertEquals(hit.score, explanation.getValue().doubleValue(), 0.000005f); Matcher m = Pattern.compile( "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), using score mode (None|Avg|Min|Max|Total)") From 26db7285f6f34b5ab5033d15adb7ae808d4ea089 Mon Sep 17 00:00:00 2001 From: Marcus Date: Tue, 2 May 2023 15:32:54 -0400 Subject: [PATCH 13/21] fixing in-progress issues. --- .../search/join/ToParentBlockJoinQuery.java | 16 ++---- .../lucene/search/join/TestBlockJoin.java | 50 ++++++++++--------- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index 588748e611af..0ac09c25b94f 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -22,6 +22,8 @@ import java.util.Collection; import java.util.Collections; import java.util.Locale; + +import org.apache.lucene.index.EmptyDocValuesProducer; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.ConstantScoreQuery; @@ -425,17 +427,9 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM // if (bestChild == null) { if (scoreMode == ScoreMode.None) { - double score = bestChild.getValue().doubleValue(); - return Explanation.match( - score, - String.format( - Locale.ROOT, - "Score based on %d child docs in range from %d to %d, using score mode %s", - matches, - start, - end, - scoreMode), - bestChild); + return Explanation.noMatch( + "No children matched"); + } else { return Explanation.match( 0.0f, diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java index 03a384cc667d..95016cf79550 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java @@ -891,30 +891,32 @@ public void testRandom() throws Exception { int childId = Integer.parseInt(document.get("childID")); // System.out.println(" hit docID=" + hit.doc + " childId=" + childId + " parentId=" + // document.get("parentID")); - assertTrue(explanation.isMatch()); - // This test is failing in strange ways. - // assertEquals(hit.score, explanation.getValue().doubleValue(), 0.000005f); - Matcher m = - Pattern.compile( - "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), using score mode (None|Avg|Min|Max|Total)") - .matcher(explanation.getDescription()); - assertTrue("Block Join description not matches", m.matches()); - assertTrue("Matched children not positive", Integer.parseInt(m.group(1)) > 0); - assertEquals( - "Wrong child range start", hit.doc - 1 - childId, Integer.parseInt(m.group(2))); - assertEquals("Wrong child range end", hit.doc - 1, Integer.parseInt(m.group(3))); - Explanation childWeightExplanation = explanation.getDetails()[0]; - if ("sum of:".equals(childWeightExplanation.getDescription())) { - childWeightExplanation = childWeightExplanation.getDetails()[0]; - } - if (agg == ScoreMode.None) { - assertTrue( - "Wrong child weight description", - childWeightExplanation.getDescription().startsWith("ConstantScore(")); - } else { - assertTrue( - "Wrong child weight description", - childWeightExplanation.getDescription().startsWith("weight(child")); + if (explanation.isMatch()) { + assertTrue(explanation.isMatch()); + // This test is failing in strange ways. + assertEquals(hit.score, explanation.getValue().doubleValue(), 0.000005f); + Matcher m = + Pattern.compile( + "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), using score mode (None|Avg|Min|Max|Total)") + .matcher(explanation.getDescription()); + assertTrue("Block Join description not matches", m.matches()); + assertTrue("Matched children not positive", Integer.parseInt(m.group(1)) > 0); + assertEquals( + "Wrong child range start", hit.doc - 1 - childId, Integer.parseInt(m.group(2))); + assertEquals("Wrong child range end", hit.doc - 1, Integer.parseInt(m.group(3))); + Explanation childWeightExplanation = explanation.getDetails()[0]; + if ("sum of:".equals(childWeightExplanation.getDescription())) { + childWeightExplanation = childWeightExplanation.getDetails()[0]; + } + if (agg == ScoreMode.None) { + assertTrue( + "Wrong child weight description", + childWeightExplanation.getDescription().startsWith("ConstantScore(")); + } else { + assertTrue( + "Wrong child weight description", + childWeightExplanation.getDescription().startsWith("weight(child")); + } } } } From b19c9f99d399bd49629bf937517804b96f7b8008 Mon Sep 17 00:00:00 2001 From: Marcus Date: Tue, 2 May 2023 16:43:44 -0400 Subject: [PATCH 14/21] adding a total explanation. --- .../search/join/ToParentBlockJoinQuery.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index 0ac09c25b94f..38e60bbe6811 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -423,8 +423,6 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM } } } - // this one causes issues - // if (bestChild == null) { if (scoreMode == ScoreMode.None) { return Explanation.noMatch( @@ -453,6 +451,19 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM end, scoreMode), bestChild); + } + if (scoreMode == ScoreMode.Total) { + double totalScore = childScoreSum; + return Explanation.match( + totalScore, + String.format( + Locale.ROOT, + "Score based on %d child docs in range from %d to %d, using score mode %s", + matches, + start, + end, + scoreMode), + bestChild); } else { return Explanation.match( score(), From 1fde482fec82a3d3aa2e851cb08e1c72f77f6a43 Mon Sep 17 00:00:00 2001 From: Marcus Date: Tue, 2 May 2023 17:21:25 -0400 Subject: [PATCH 15/21] fix the control flow and comment out helpful debug aid for others in the future. --- .../search/join/ToParentBlockJoinQuery.java | 26 ++++--------------- .../lucene/search/join/TestBlockJoin.java | 20 +++++++------- 2 files changed, 16 insertions(+), 30 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index 38e60bbe6811..ce823118b536 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -22,8 +22,6 @@ import java.util.Collection; import java.util.Collections; import java.util.Locale; - -import org.apache.lucene.index.EmptyDocValuesProducer; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.ConstantScoreQuery; @@ -413,10 +411,6 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM matches++; childScoreSum += child.getValue().doubleValue(); - if (scoreMode == ScoreMode.Total) { - continue; - } - if (bestChild == null || child.getValue().doubleValue() > bestChild.getValue().doubleValue()) { bestChild = child; @@ -425,8 +419,7 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM } if (bestChild == null) { if (scoreMode == ScoreMode.None) { - return Explanation.noMatch( - "No children matched"); + return Explanation.noMatch("No children matched"); } else { return Explanation.match( @@ -452,21 +445,10 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM scoreMode), bestChild); } - if (scoreMode == ScoreMode.Total) { + if (scoreMode == ScoreMode.Total && matches > 0) { double totalScore = childScoreSum; return Explanation.match( - totalScore, - String.format( - Locale.ROOT, - "Score based on %d child docs in range from %d to %d, using score mode %s", - matches, - start, - end, - scoreMode), - bestChild); - } else { - return Explanation.match( - score(), + totalScore, String.format( Locale.ROOT, "Score based on %d child docs in range from %d to %d, using score mode %s", @@ -475,6 +457,8 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM end, scoreMode), bestChild); + } else { + return Explanation.noMatch("Unexpected score mode: " + scoreMode); } } } diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java index 95016cf79550..37415468b99e 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java @@ -893,16 +893,18 @@ public void testRandom() throws Exception { // document.get("parentID")); if (explanation.isMatch()) { assertTrue(explanation.isMatch()); + // System.out.println("explanation.getDescription: " + + // explanation.getDescription()); // This test is failing in strange ways. - assertEquals(hit.score, explanation.getValue().doubleValue(), 0.000005f); + assertEquals(hit.score, explanation.getValue().doubleValue(), 0.005f); Matcher m = - Pattern.compile( - "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), using score mode (None|Avg|Min|Max|Total)") - .matcher(explanation.getDescription()); + Pattern.compile( + "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), using score mode (None|Avg|Min|Max|Total)") + .matcher(explanation.getDescription()); assertTrue("Block Join description not matches", m.matches()); assertTrue("Matched children not positive", Integer.parseInt(m.group(1)) > 0); assertEquals( - "Wrong child range start", hit.doc - 1 - childId, Integer.parseInt(m.group(2))); + "Wrong child range start", hit.doc - 1 - childId, Integer.parseInt(m.group(2))); assertEquals("Wrong child range end", hit.doc - 1, Integer.parseInt(m.group(3))); Explanation childWeightExplanation = explanation.getDetails()[0]; if ("sum of:".equals(childWeightExplanation.getDescription())) { @@ -910,12 +912,12 @@ public void testRandom() throws Exception { } if (agg == ScoreMode.None) { assertTrue( - "Wrong child weight description", - childWeightExplanation.getDescription().startsWith("ConstantScore(")); + "Wrong child weight description", + childWeightExplanation.getDescription().startsWith("ConstantScore(")); } else { assertTrue( - "Wrong child weight description", - childWeightExplanation.getDescription().startsWith("weight(child")); + "Wrong child weight description", + childWeightExplanation.getDescription().startsWith("weight(child")); } } } From 3d23e2e018f71439c5c035d35787a3a6d9f5a234 Mon Sep 17 00:00:00 2001 From: Marcus Date: Wed, 3 May 2023 12:10:07 -0400 Subject: [PATCH 16/21] add min and max. --- lucene/CHANGES.txt | 3 +- .../search/join/ToParentBlockJoinQuery.java | 30 ++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index db018fb21446..73c2e5af55e3 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -118,7 +118,8 @@ New Features Improvements --------------------- -(No changes) + +GITHUB#12245: Add support for Score Mode to `ToParentBlockJoinQuery` explain. (Marcus Eagan via Mikhail Khludnev) Optimizations --------------------- diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index ce823118b536..a81334da9eb7 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -403,6 +403,7 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM int end = context.docBase + parentApproximation.docID() - 1; // -1 b/c parentDoc is parent doc Explanation bestChild = null; + Explanation worstChild = null; double childScoreSum = 0; int matches = 0; for (int childDoc = start; childDoc <= end; childDoc++) { @@ -415,12 +416,15 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM || child.getValue().doubleValue() > bestChild.getValue().doubleValue()) { bestChild = child; } + if (worstChild == null + || child.getValue().doubleValue() < worstChild.getValue().doubleValue()) { + worstChild = child; + } } } if (bestChild == null) { if (scoreMode == ScoreMode.None) { return Explanation.noMatch("No children matched"); - } else { return Explanation.match( 0.0f, @@ -457,6 +461,30 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM end, scoreMode), bestChild); + } + if (scoreMode == ScoreMode.Max && matches > 0) { + return Explanation.match( + bestChild.getValue().doubleValue(), + String.format( + Locale.ROOT, + "Score based on %d child docs in range from %d to %d, using score mode %s", + matches, + start, + end, + scoreMode), + bestChild); + } + if (scoreMode == ScoreMode.Min && matches > 0) { + return Explanation.match( + worstChild.getValue().doubleValue(), + String.format( + Locale.ROOT, + "Score based on %d child docs in range from %d to %d, using score mode %s", + matches, + start, + end, + scoreMode), + worstChild); } else { return Explanation.noMatch("Unexpected score mode: " + scoreMode); } From 94cb3ac5ecaea2b1506309094ca99573124273f9 Mon Sep 17 00:00:00 2001 From: Marcus Date: Thu, 4 May 2023 13:42:46 -0400 Subject: [PATCH 17/21] refactor to use switch statement. --- .../search/join/ToParentBlockJoinQuery.java | 109 ++++++++---------- 1 file changed, 45 insertions(+), 64 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index a81334da9eb7..3b8cf1002813 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -410,8 +410,6 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM Explanation child = childWeight.explain(context, childDoc - context.docBase); if (child.isMatch()) { matches++; - childScoreSum += child.getValue().doubleValue(); - if (bestChild == null || child.getValue().doubleValue() > bestChild.getValue().doubleValue()) { bestChild = child; @@ -422,72 +420,55 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM } } } + if (bestChild == null) { - if (scoreMode == ScoreMode.None) { - return Explanation.noMatch("No children matched"); - } else { - return Explanation.match( - 0.0f, - String.format( - Locale.ROOT, - "Score based on 0 child docs in range from %d to %d, using score mode %s", - start, - end, - scoreMode)); + switch (scoreMode) { + case None: + return Explanation.noMatch("No children matched"); + default: + return Explanation.match( + this.score(), formatScoreExplanation(0, start, end, scoreMode)); } } - if (scoreMode == ScoreMode.Avg) { - double avgScore = matches > 0 ? childScoreSum / (double) matches : 0; - return Explanation.match( - avgScore, - String.format( - Locale.ROOT, - "Score based on %d child docs in range from %d to %d, using score mode %s", - matches, - start, - end, - scoreMode), - bestChild); - } - if (scoreMode == ScoreMode.Total && matches > 0) { - double totalScore = childScoreSum; - return Explanation.match( - totalScore, - String.format( - Locale.ROOT, - "Score based on %d child docs in range from %d to %d, using score mode %s", - matches, - start, - end, - scoreMode), - bestChild); - } - if (scoreMode == ScoreMode.Max && matches > 0) { - return Explanation.match( - bestChild.getValue().doubleValue(), - String.format( - Locale.ROOT, - "Score based on %d child docs in range from %d to %d, using score mode %s", - matches, - start, - end, - scoreMode), - bestChild); - } - if (scoreMode == ScoreMode.Min && matches > 0) { - return Explanation.match( - worstChild.getValue().doubleValue(), - String.format( - Locale.ROOT, - "Score based on %d child docs in range from %d to %d, using score mode %s", - matches, - start, - end, - scoreMode), - worstChild); - } else { - return Explanation.noMatch("Unexpected score mode: " + scoreMode); + + switch (scoreMode) { + case Avg: + double avgScore = matches > 0 ? childScoreSum / (double) matches : 0; + return Explanation.match( + this.score(), formatScoreExplanation(matches, start, end, scoreMode), bestChild); + case Total: + if (matches > 0) { + return Explanation.match( + this.score(), formatScoreExplanation(matches, start, end, scoreMode), bestChild); + } + break; + case Max: + if (matches > 0) { + return Explanation.match( + this.score(), formatScoreExplanation(matches, start, end, scoreMode), bestChild); + } + break; + case Min: + if (matches > 0) { + return Explanation.match( + this.score(), formatScoreExplanation(matches, start, end, scoreMode), worstChild); + } + break; + default: + return Explanation.noMatch("Unexpected score mode: " + scoreMode); } + + return Explanation.noMatch("No matches found"); + } + + private String formatScoreExplanation(int matches, int start, int end, ScoreMode scoreMode) { + return String.format( + Locale.ROOT, + "Score based on %d child docs in range from %d to %d, using score mode %s", + matches, + start, + end, + scoreMode); } } From 824a86e385ac504509fc7d6a067587df234c1c32 Mon Sep 17 00:00:00 2001 From: Marcus Date: Thu, 4 May 2023 17:46:59 -0400 Subject: [PATCH 18/21] Adding more support for none and removing score math. --- .../org/apache/lucene/search/join/ToParentBlockJoinQuery.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index 3b8cf1002813..19ecc68f2f15 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -424,7 +424,8 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM if (bestChild == null) { switch (scoreMode) { case None: - return Explanation.noMatch("No children matched"); + return Explanation.match( + this.score(), formatScoreExplanation(0, start, end, scoreMode), bestChild); default: return Explanation.match( this.score(), formatScoreExplanation(0, start, end, scoreMode)); @@ -433,7 +434,6 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM switch (scoreMode) { case Avg: - double avgScore = matches > 0 ? childScoreSum / (double) matches : 0; return Explanation.match( this.score(), formatScoreExplanation(matches, start, end, scoreMode), bestChild); case Total: From d3f413e3c4d8d894a12b485aae82f89504104358 Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Fri, 5 May 2023 17:13:22 +0300 Subject: [PATCH 19/21] Adding a bit if simplicity. --- .../search/join/ToParentBlockJoinQuery.java | 45 +++---------------- .../lucene/search/join/TestBlockJoin.java | 10 +---- 2 files changed, 6 insertions(+), 49 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index 19ecc68f2f15..4a3f445fbd39 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -404,7 +404,7 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM Explanation bestChild = null; Explanation worstChild = null; - double childScoreSum = 0; + int matches = 0; for (int childDoc = start; childDoc <= end; childDoc++) { Explanation child = childWeight.explain(context, childDoc - context.docBase); @@ -420,45 +420,10 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM } } } - - if (bestChild == null) { - switch (scoreMode) { - case None: - return Explanation.match( - this.score(), formatScoreExplanation(0, start, end, scoreMode), bestChild); - default: - return Explanation.match( - this.score(), formatScoreExplanation(0, start, end, scoreMode)); - } - } - - switch (scoreMode) { - case Avg: - return Explanation.match( - this.score(), formatScoreExplanation(matches, start, end, scoreMode), bestChild); - case Total: - if (matches > 0) { - return Explanation.match( - this.score(), formatScoreExplanation(matches, start, end, scoreMode), bestChild); - } - break; - case Max: - if (matches > 0) { - return Explanation.match( - this.score(), formatScoreExplanation(matches, start, end, scoreMode), bestChild); - } - break; - case Min: - if (matches > 0) { - return Explanation.match( - this.score(), formatScoreExplanation(matches, start, end, scoreMode), worstChild); - } - break; - default: - return Explanation.noMatch("Unexpected score mode: " + scoreMode); - } - - return Explanation.noMatch("No matches found"); + assert matches>0 : "No matches should be handled before."; + return Explanation.match( + this.score(), formatScoreExplanation(matches, start, end, scoreMode), + scoreMode==ScoreMode.Min ? worstChild:bestChild); } private String formatScoreExplanation(int matches, int start, int end, ScoreMode scoreMode) { diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java index 37415468b99e..596174c909c0 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java @@ -889,14 +889,7 @@ public void testRandom() throws Exception { Explanation explanation = joinS.explain(childJoinQuery, hit.doc); Document document = joinS.storedFields().document(hit.doc - 1); int childId = Integer.parseInt(document.get("childID")); - // System.out.println(" hit docID=" + hit.doc + " childId=" + childId + " parentId=" + - // document.get("parentID")); - if (explanation.isMatch()) { - assertTrue(explanation.isMatch()); - // System.out.println("explanation.getDescription: " + - // explanation.getDescription()); - // This test is failing in strange ways. - assertEquals(hit.score, explanation.getValue().doubleValue(), 0.005f); + assertEquals(hit.score, explanation.getValue().floatValue(), 0.1f); Matcher m = Pattern.compile( "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), using score mode (None|Avg|Min|Max|Total)") @@ -919,7 +912,6 @@ public void testRandom() throws Exception { "Wrong child weight description", childWeightExplanation.getDescription().startsWith("weight(child")); } - } } } From f52ebecb7f09142bfb1b839b0f7f13ce39c7e12b Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Fri, 5 May 2023 18:12:13 +0300 Subject: [PATCH 20/21] some tidysh --- .../search/join/ToParentBlockJoinQuery.java | 8 ++-- .../lucene/search/join/TestBlockJoin.java | 46 +++++++++---------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java index 4a3f445fbd39..6134cd0e1dde 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java @@ -420,10 +420,12 @@ public Explanation explain(LeafReaderContext context, Weight childWeight, ScoreM } } } - assert matches>0 : "No matches should be handled before."; + assert matches > 0 : "No matches should be handled before."; + Explanation subExplain = scoreMode == ScoreMode.Min ? worstChild : bestChild; return Explanation.match( - this.score(), formatScoreExplanation(matches, start, end, scoreMode), - scoreMode==ScoreMode.Min ? worstChild:bestChild); + this.score(), + formatScoreExplanation(matches, start, end, scoreMode), + subExplain == null ? Collections.emptyList() : Collections.singleton(subExplain)); } private String formatScoreExplanation(int matches, int start, int end, ScoreMode scoreMode) { diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java index 596174c909c0..1ee633758abe 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java @@ -889,29 +889,29 @@ public void testRandom() throws Exception { Explanation explanation = joinS.explain(childJoinQuery, hit.doc); Document document = joinS.storedFields().document(hit.doc - 1); int childId = Integer.parseInt(document.get("childID")); - assertEquals(hit.score, explanation.getValue().floatValue(), 0.1f); - Matcher m = - Pattern.compile( - "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), using score mode (None|Avg|Min|Max|Total)") - .matcher(explanation.getDescription()); - assertTrue("Block Join description not matches", m.matches()); - assertTrue("Matched children not positive", Integer.parseInt(m.group(1)) > 0); - assertEquals( - "Wrong child range start", hit.doc - 1 - childId, Integer.parseInt(m.group(2))); - assertEquals("Wrong child range end", hit.doc - 1, Integer.parseInt(m.group(3))); - Explanation childWeightExplanation = explanation.getDetails()[0]; - if ("sum of:".equals(childWeightExplanation.getDescription())) { - childWeightExplanation = childWeightExplanation.getDetails()[0]; - } - if (agg == ScoreMode.None) { - assertTrue( - "Wrong child weight description", - childWeightExplanation.getDescription().startsWith("ConstantScore(")); - } else { - assertTrue( - "Wrong child weight description", - childWeightExplanation.getDescription().startsWith("weight(child")); - } + assertEquals(hit.score, explanation.getValue().floatValue(), 0.1f); + Matcher m = + Pattern.compile( + "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), using score mode (None|Avg|Min|Max|Total)") + .matcher(explanation.getDescription()); + assertTrue("Block Join description not matches", m.matches()); + assertTrue("Matched children not positive", Integer.parseInt(m.group(1)) > 0); + assertEquals( + "Wrong child range start", hit.doc - 1 - childId, Integer.parseInt(m.group(2))); + assertEquals("Wrong child range end", hit.doc - 1, Integer.parseInt(m.group(3))); + Explanation childWeightExplanation = explanation.getDetails()[0]; + if ("sum of:".equals(childWeightExplanation.getDescription())) { + childWeightExplanation = childWeightExplanation.getDetails()[0]; + } + if (agg == ScoreMode.None) { + assertTrue( + "Wrong child weight description", + childWeightExplanation.getDescription().startsWith("ConstantScore(")); + } else { + assertTrue( + "Wrong child weight description", + childWeightExplanation.getDescription().startsWith("weight(child")); + } } } From b98018d7649613f9e2488a90f79455e07c67a654 Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Fri, 5 May 2023 18:31:37 +0300 Subject: [PATCH 21/21] restore assert --- .../src/test/org/apache/lucene/search/join/TestBlockJoin.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java index 1ee633758abe..ca4246196fce 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoin.java @@ -889,7 +889,8 @@ public void testRandom() throws Exception { Explanation explanation = joinS.explain(childJoinQuery, hit.doc); Document document = joinS.storedFields().document(hit.doc - 1); int childId = Integer.parseInt(document.get("childID")); - assertEquals(hit.score, explanation.getValue().floatValue(), 0.1f); + assertTrue(explanation.isMatch()); + assertEquals(hit.score, explanation.getValue().doubleValue(), 0.0f); Matcher m = Pattern.compile( "Score based on ([0-9]+) child docs in range from ([0-9]+) to ([0-9]+), using score mode (None|Avg|Min|Max|Total)")