Removed Scorer#getWeight

[iamsanjay]

Description

Closes #13410

If Caller requires Weight then they have to keep track of Weight with which Scorer was created in the first place instead of relying on Scorer as @jpountz mentioned in the comment as well.

[iamsanjay]

There is one issue that I have faced. To get reference of weight instance in Collector instance, one can override below method.

default void setWeight(Weight weight) {}

For BooleanWeight instance, I tried to create a Map.of(Scorer, Query) in above method and then use it setScorer to fetch the query as getWeight is not available now and therefore no way to fetch the respective query,

Map<Scorer, Query> mapScQy= new HashMap<Scorer, Query>();
          if(weight instanceof BooleanWeight bw){
            for(BooleanWeight.WeightedBooleanClause clause: bw.weightedClauses){
              mapScQy.put(clause.weight.scorer(context), clause.clause.query());
            }
          }

However, the scorer object that I found inside BooleanWeight#WeightedBooleanClause and the one with which setScorer is called, both are different. My point is that yes you can have get access to weight instance, However, It is not possible to fetch the Query for which that Scorer was created. IMO, It's not possible to create the Map and then use it in collect method.

Consequently I removed TestSubScorerFreqs.java and some other parts of code from the other test cases.

[jpountz]

Could you also look into removing Weight from the Scorer constructor? To me this is the most annoying bit.

Regarding TestSubScorerFreqs, does using ScorerIndexSearcher rather than IndexSearcher help get scorer instances that match the Scorable?

[iamsanjay]

Could you also look into removing Weight from the Scorer constructor? To me this is the most annoying bit.

Yup I am on it! Total 49 usages as per quick IntelliJ search.

Regarding TestSubScorerFreqs, does using ScorerIndexSearcher rather than IndexSearcher help get scorer instances that match the Scorable?

Okay, I will check If switching to ScorerIndexSearcher can help me getting same Scorer. Thanks!

[iamsanjay]

@jpountz We are also removing weight from Subclasses of Scorer as well, right? Because I already removed from good amount of classes such as ConstantScoreScorer and few others. Just want to make sure before moving ahead.

[iamsanjay]

1. There were three classes that are using weight in some way so I leave them to be for now.

   FunctionQuery#AllScorer
   

   lucene/lucene/queries/src/java/org/apache/lucene/queries/function/FunctionQuery.java

   Line 117 in 750a7c4

   vals = func.getValues(weight.context, context);

   TermAutomatonScorer
   

   lucene/lucene/sandbox/src/java/org/apache/lucene/sandbox/search/TermAutomatonScorer.java

   Line 68 in 750a7c4

   this.runAutomaton = new TermRunAutomaton(weight.automaton, subs.length);

   TestMinShouldMatch2#SlowMinShouldMatchScorer
   

   lucene/lucene/core/src/test/org/apache/lucene/search/TestMinShouldMatch2.java

   Line 357 in 750a7c4

   BooleanQuery bq = (BooleanQuery) weight.getQuery();

2. As I removed weight, few ctors -- JustCompileScorer, SimpleScorer, TestScoreCachingWrappingScorer#SimpleScorer, Scorer -- have been reduced to default ctor. Should I removed those as well?

3. As we removing Weight, some APIs that were calling with weight is not required. Hence I removed the weight from their method signature as well -- Check: FunctionValues#getRangeScorer and all the respective overrides.

4. SpatialQuery has also one method where we could possibly change the API to remove the weight parameter as it is not required now. Should I change that as well? There is a possibility we may found some more APIs where the weight can be removed now.

[jpountz]

We are also removing weight from Subclasses of Scorer as well, right?

Yes, that would be great. If there are a few sub classes that really need a Weight, we could keep it, it looks like you identitfied a few of them already.

[jpountz]

few ctors have been reduced to default ctor. Should I removed those as well?

For those that are in the java folder, I'd keep them and add javadocs (I thought the build checked that). For those under the test folder, we can remove.

Check: FunctionValues#getRangeScorer

Good question. Your change looks good this way, but I know Solr is a heavy user of FunctionValues, I'm curious if that would be annoying to them. @gerlowskija @dsmiley @HoustonPutman Do you have an opinion on this?

SpatialQuery has also one method where we could possibly change the API to remove the weight parameter as it is not required now. Should I change that as well?

Yes. It looks like it was only there to be able to pass the Weight to a Scorer constructor.

[dsmiley]

It's not clear why Solr would care with regards to FunctionValues & Weights in particular. I don't notice Solr using Weights there but maybe I'm not looking in quite the right spot?

if Scorer.getWeight is being removed, I only see a couple places in Solr where this is used, both in LTR, interestingly. This shouldn't stop Lucene from doing what it ought to do.

[jpountz]

It's not clear why Solr would care with regards to FunctionValues & Weights in particular. I don't notice Solr using Weights there but maybe I'm not looking in quite the right spot?

Maybe it doesn't use them, I was just trying to be careful since I believe that Solr is using the FunctionValues API a lot for custom scoring/sorting.

This shouldn't stop Lucene from doing what it ought to do.

Thanks for the guidance, this is the sort of feedback I was looking for.

[jpountz]

LGTM

[jpountz]

Undo the new line?

[jpountz]

Thank you!