jpountz commented on code in PR #13587:
URL: https://github.com/apache/lucene/pull/13587#discussion_r1684532729
##########
lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java:
##########
@@ -16,23 +16,16 @@
*/
package org.apache.lucene.search.join;
+import static org.apache.lucene.search.ScoreMode.TOP_SCORES;
+
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
-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.ScoreMode;
-import org.apache.lucene.search.Scorer;
-import org.apache.lucene.search.ScorerSupplier;
-import org.apache.lucene.search.TermQuery;
-import org.apache.lucene.search.Weight;
+import org.apache.lucene.index.*;
+import org.apache.lucene.search.*;
Review Comment:
We prefer to avoid wildcard imports.
##########
lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java:
##########
@@ -118,4 +110,114 @@ public void testScoreNone() throws IOException {
reader.close();
dir.close();
}
+
+ public void testScoreMax() throws IOException {
+ try (Directory dir = newDirectory()) {
+ try (RandomIndexWriter w =
+ new RandomIndexWriter(
+ random(),
+ dir,
+ newIndexWriterConfig()
+ .setMergePolicy(
+ // retain doc id order
+ newLogMergePolicy(random().nextBoolean())))) {
+
+ for (String[][] values :
+ Arrays.asList(
+ new String[][] {{"A", "B"}, {"A", "B", "C"}},
+ new String[][] {{"A"}, {"B"}},
+ new String[][] {{}},
+ new String[][] {{"A", "B", "C"}, {"A", "B", "C", "D"}},
+ new String[][] {{"B"}},
+ new String[][] {{"B", "C"}, {"A", "B"}, {"A", "C"}})) {
+
+ List<Document> docs = new ArrayList<>();
+ for (String[] value : values) {
+ Document childDoc = new Document();
+ childDoc.add(newStringField("type", "child", Field.Store.NO));
+ for (String v : value) {
+ childDoc.add(newStringField("value", v, Field.Store.NO));
+ }
+ docs.add(childDoc);
+ }
+
+ Document parentDoc = new Document();
+ parentDoc.add(newStringField("type", "parent", Field.Store.NO));
+ docs.add(parentDoc);
+
+ w.addDocuments(docs);
+ }
+
+ w.forceMerge(1);
+ }
+
+ try (IndexReader reader = DirectoryReader.open(dir)) {
+ IndexSearcher searcher = newSearcher(reader);
+
+ BooleanQuery childQuery =
+ new BooleanQuery.Builder()
+ .add(
+ new BoostQuery(
+ new ConstantScoreQuery(new TermQuery(new Term("value",
"A"))), 2),
+ BooleanClause.Occur.SHOULD)
+ .add(
+ new ConstantScoreQuery(new TermQuery(new Term("value",
"B"))),
+ BooleanClause.Occur.SHOULD)
+ .add(
+ new BoostQuery(
+ new ConstantScoreQuery(new TermQuery(new Term("value",
"C"))), 3),
+ BooleanClause.Occur.SHOULD)
+ .add(
+ new BoostQuery(
+ new ConstantScoreQuery(new TermQuery(new Term("value",
"D"))), 4),
+ BooleanClause.Occur.SHOULD)
+ .setMinimumNumberShouldMatch(2)
Review Comment:
I wonder if you initially set it because the produced scorer would otherwise
not skip properly? If this was the reason, the fact that you now propagate
`setTopLevelScoringClause` should hopefully fix the root cause without
requiring a min should match to be configured.
##########
lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java:
##########
@@ -155,6 +159,11 @@ public Scorer get(long leadCost) throws IOException {
public long cost() {
return childScorerSupplier.cost();
}
+
+ @Override
+ public void setTopLevelScoringClause() throws IOException {
+ childScorerSupplier.setTopLevelScoringClause();
Review Comment:
The name of this method is not great, but it's essentially about whether
we'll call `setMinCompetitiveScore` on the scorer, so we should only propagate
if the scoreMode is MAX.
##########
lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java:
##########
@@ -101,12 +101,16 @@ public Weight createWeight(
.rewrite(new ConstantScoreQuery(childQuery))
.createWeight(searcher, weightScoreMode, 0f);
} else {
- // if the score is needed we force the collection mode to COMPLETE
because the child query
- // cannot skip
- // non-competitive documents.
+ // if the score is needed and the score mode is not max, we force the
collection mode to
+ // COMPLETE because the
+ // child query cannot skip non-competitive documents.
childWeight =
childQuery.createWeight(
- searcher, weightScoreMode.needsScores() ? COMPLETE :
weightScoreMode, boost);
+ searcher,
+ weightScoreMode.needsScores() && childScoreMode != ScoreMode.Max
Review Comment:
this sounds good to me, maybe add a comment about it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
