jpountz commented on code in PR #13587: URL: https://github.com/apache/lucene/pull/13587#discussion_r1683500985
########## lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java: ########## @@ -101,12 +99,7 @@ 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. - childWeight = - childQuery.createWeight( - searcher, weightScoreMode.needsScores() ? COMPLETE : weightScoreMode, boost); + childWeight = childQuery.createWeight(searcher, weightScoreMode, boost); Review Comment: Should we keep using COMPLETE if the score mode is not max since we wouldn't be propagating the min competitive score? ########## lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java: ########## @@ -118,4 +110,108 @@ public void testScoreNone() throws IOException { reader.close(); dir.close(); } + + public void testScoreMax() throws IOException { + try (Directory dir = newDirectory()) { + // retain doc id order + 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"}, + new String[] {"A"}, + new String[] {}, + new String[] {"A", "B", "C"}, + new String[] {"B"}, + new String[] {"B", "C"})) { + List<Document> docs = new ArrayList<>(); + + Document childDoc = new Document(); + childDoc.add(newStringField("type", "child", Field.Store.NO)); + for (String value : values) { + childDoc.add(newStringField("value", value, Field.Store.NO)); + } Review Comment: The test would be more interesting if there were multiple children per parent that produce different scores. Here you are always creating a single child document per parent. ########## lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinScorer.java: ########## @@ -118,4 +110,108 @@ public void testScoreNone() throws IOException { reader.close(); dir.close(); } + + public void testScoreMax() throws IOException { + try (Directory dir = newDirectory()) { + // retain doc id order + 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"}, + new String[] {"A"}, + new String[] {}, + new String[] {"A", "B", "C"}, + new String[] {"B"}, + new String[] {"B", "C"})) { + List<Document> docs = new ArrayList<>(); + + Document childDoc = new Document(); + childDoc.add(newStringField("type", "child", Field.Store.NO)); + for (String value : values) { + childDoc.add(newStringField("value", value, 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) + .setMinimumNumberShouldMatch(2) + .build(); + BitSetProducer parentsFilter = + new QueryBitSetProducer(new TermQuery(new Term("type", "parent"))); + ToParentBlockJoinQuery parentQuery = + new ToParentBlockJoinQuery(childQuery, parentsFilter, ScoreMode.Max); + + Weight weight = searcher.createWeight(searcher.rewrite(parentQuery), TOP_SCORES, 1); + ScorerSupplier ss = weight.scorerSupplier(searcher.getIndexReader().leaves().get(0)); + ss.setTopLevelScoringClause(); Review Comment: We probably need to implement this method on BlockJoinScorer to propagate to the inner clause when the score mode is max. -- 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