Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support getMaxScore of ConjunctionScorer for non top level scoring clause #13043

Merged
merged 2 commits into from Jan 29, 2024

Conversation

mrkm4ntr
Copy link
Contributor

Description

After introducing topLevelScoringClause, ConjunctionScorer with multiple scorers can be used for non top level scoring clause conjunctions instead of BlockMaxConjunctionScorer even requiredScorers is empty. In such case, ConjunctionScorer returns Infinity as maxScore and it ruins some optimizations like parent WANDScorer.
https://github.com/apache/lucene/blob/7d35ae485807147460f63ea58ae495124e972e13/lucene/core/src/java/org/apache/lucene/search/Boolean2ScorerSupplier.java#L218C77-L218C98

@mrkm4ntr mrkm4ntr force-pushed the conjunction-scorer-max-score branch 2 times, most recently from 85edffd to 608a334 Compare January 29, 2024 02:36
@jpountz
Copy link
Contributor

jpountz commented Jan 29, 2024

Thanks for looking into this, can you explain what kind of queries perform better with this change?

@mrkm4ntr
Copy link
Contributor Author

For this case. Suppose ScorerA, B, and C can return valid maxScore. If the ScorerD is dominant, larger minCompetitiveScore is set to WANDScorer. But ConjunctionScorer returns Infinity as maxScore and scaledMaxScore becomes 0. Then WANDScorer cannot skip docs well. Is my understanding correct?

DisjunctionMaxScorer(scorers=[
  WANDScorer(scorers=[ConjunctionScorer(ScorerA, ScorerB), ScorerC]),
  ScorerD
], tieBreakerMultiplier=0.0)

@jpountz
Copy link
Contributor

jpountz commented Jan 29, 2024

OK, I see, it's about conjunctions within disjunctions. Thanks for explaining.

@jpountz
Copy link
Contributor

jpountz commented Jan 29, 2024

The change looks good to me, can you add a CHANGES entry?

@mrkm4ntr
Copy link
Contributor Author

Thanks. 9.10 as well?

@jpountz
Copy link
Contributor

jpountz commented Jan 29, 2024

Yes please.

@mrkm4ntr
Copy link
Contributor Author

Added, thanks.

@jpountz jpountz merged commit 1414136 into apache:main Jan 29, 2024
4 checks passed
@jpountz jpountz added this to the 9.10.0 milestone Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants