SOLR-9494: CollapseQParser's collectors should override needsScores(); can trigger exceptions

Also, field cscore was not needed.

solr/CHANGES.txt

@@ -154,6 +154,16 @@ Other Changes
 
 * SOLR-9498: Remove HDFS properties from DIH solrconfig.xml, as started in SOLR-6943 (Alexandre Rafalovitch)
 
+
+==================  6.2.1 ==================
+
+Bug Fixes
+----------------------
+
+* SOLR-9494: Use of {!collapse} sometimes doesn't correctly return true for Collector.needsScores(), especially when the
+  query was cached. This can cause an exception when 'q' is a SpanQuery or potentially others. (David Smiley)
+
+
 ==================  6.2.0 ==================
 
 Versions of Major Components

solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java

@@ -517,6 +517,8 @@ public class CollapsingQParserPlugin extends QParserPlugin {
       }
     }
 
+    @Override public boolean needsScores() { return true; }
+
     @Override
     protected void doSetNextReader(LeafReaderContext context) throws IOException {
       this.contexts[context.ord] = context;
@@ -726,6 +728,8 @@ public class CollapsingQParserPlugin extends QParserPlugin {
 
     }
 
+    @Override public boolean needsScores() { return true; }
+
     @Override
     protected void doSetNextReader(LeafReaderContext context) throws IOException {
       this.contexts[context.ord] = context;
@@ -909,6 +913,8 @@ public class CollapsingQParserPlugin extends QParserPlugin {
       }
     }
 
+    @Override public boolean needsScores() { return needsScores || super.needsScores(); }
+
     public void setScorer(Scorer scorer) {
       this.collapseStrategy.setScorer(scorer);
     }
@@ -1069,6 +1075,8 @@ public class CollapsingQParserPlugin extends QParserPlugin {
       }
     }
 
+    @Override public boolean needsScores() { return needsScores || super.needsScores(); }
+
     public void setScorer(Scorer scorer) {
       this.collapseStrategy.setScorer(scorer);
     }
@@ -1686,7 +1694,6 @@ public class CollapsingQParserPlugin extends QParserPlugin {
     private float[] ordVals;
     private Map rcontext;
     private final CollapseScore collapseScore = new CollapseScore();
-    private final boolean cscore;
     private float score;
 
     public OrdValueSourceStrategy(int maxDoc,
@@ -1714,7 +1721,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
         Arrays.fill(ordVals, Float.MAX_VALUE);
       }
 
-      this.cscore = collapseScore.setupIfNeeded(groupHeadSelector, rcontext);
+      collapseScore.setupIfNeeded(groupHeadSelector, rcontext);
 
       if(this.needsScores) {
         this.scores = new float[ords.length];
@@ -1735,7 +1742,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
         this.boostDocs.add(globalDoc);
       }
 
-      if(needsScores || cscore) {
+      if (needsScores) {
         this.score = scorer.score();
         this.collapseScore.score = score;
       }
@@ -2208,7 +2215,6 @@ public class CollapsingQParserPlugin extends QParserPlugin {
     private FunctionValues functionValues;
     private Map rcontext;
     private final CollapseScore collapseScore = new CollapseScore();
-    private final boolean cscore;
     private float score;
     private int index=-1;
 
@@ -2240,7 +2246,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
         comp = new MinFloatComp();
       }
 
-      this.cscore = collapseScore.setupIfNeeded(groupHeadSelector, rcontext);
+      collapseScore.setupIfNeeded(groupHeadSelector, rcontext);
 
       if(needsScores) {
         this.scores = new float[size];
@@ -2263,7 +2269,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
         return;
       }
 
-      if(needsScores || cscore) {
+      if (needsScores) {
         this.score = scorer.score();
         this.collapseScore.score = score;
       }

solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java

@@ -263,6 +263,23 @@ public class TestCollapseQParserPlugin extends SolrTestCaseJ4 {
 
   }
 
+  @Test // https://issues.apache.org/jira/browse/SOLR-9494
+  public void testNeedsScoreBugFixed() throws Exception {
+    String[] doc = {"id","1", "group_s", "xyz", "text_ws", "hello xxx world"};
+    assertU(adoc(doc));
+    assertU(commit());
+
+    ModifiableSolrParams params = params(
+        "q", "{!surround df=text_ws} 2W(hello, world)", // a SpanQuery that matches
+        "fq", "{!collapse field=group_s}", // collapse on some field
+        // note: rows= whatever; doesn't matter
+        "facet", "true", // facet on something
+        "facet.field", "group_s"
+    );
+    assertQ(req(params));
+    assertQ(req(params)); // fails *second* time!
+  }
+
   @Test
   public void testMergeBoost() throws Exception {