SOLR-17296: fix "explain" bugs with rerank scaling + multi-shard

solr/CHANGES.txt

@@ -147,6 +147,11 @@ Other Changes
 
 * GITHUB#2454: Refactor preparePutOrPost method in HttpJdkSolrClient (Andy Webb)
 
+==================  9.6.1 ==================
+Bug Fixes
+---------------------
+* SOLR-17296: Remove (broken) singlePass attempt when reRankScale + debug is used, and fix underlying NPE. (hossman)
+
 ==================  9.6.0 ==================
 New Features
 ---------------------

solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java

@@ -89,7 +89,6 @@
 import org.apache.solr.search.QueryResult;
 import org.apache.solr.search.QueryUtils;
 import org.apache.solr.search.RankQuery;
-import org.apache.solr.search.ReRankQParserPlugin;
 import org.apache.solr.search.ReturnFields;
 import org.apache.solr.search.SolrIndexSearcher;
 import org.apache.solr.search.SolrReturnFields;
@@ -782,7 +781,6 @@ protected void createMainQuery(ResponseBuilder rb) {
     boolean distribSinglePass = rb.req.getParams().getBool(ShardParams.DISTRIB_SINGLE_PASS, false);
 
     if (distribSinglePass
-        || singlePassExplain(rb.req.getParams())
         || (fields != null
             && fields.wantsField(keyFieldName)
             && fields.getRequestedFieldNames() != null
@@ -864,36 +862,6 @@ protected void createMainQuery(ResponseBuilder rb) {
     rb.addRequest(this, sreq);
   }
 
-  private boolean singlePassExplain(SolrParams params) {
-
-    /*
-     * Currently there is only one explain that requires a single pass
-     * and that is the reRank when scaling is used.
-     */
-
-    String rankQuery = params.get(CommonParams.RQ);
-    if (rankQuery != null) {
-      if (rankQuery.contains(ReRankQParserPlugin.RERANK_MAIN_SCALE)
-          || rankQuery.contains(ReRankQParserPlugin.RERANK_SCALE)) {
-        boolean debugQuery = params.getBool(CommonParams.DEBUG_QUERY, false);
-        if (debugQuery) {
-          return true;
-        } else {
-          String[] debugParams = params.getParams(CommonParams.DEBUG);
-          if (debugParams != null) {
-            for (String debugParam : debugParams) {
-              if (debugParam.equals("true")) {
-                return true;
-              }
-            }
-          }
-        }
-      }
-    }
-
-    return false;
-  }
-
   protected boolean addFL(StringBuilder fl, String field, boolean additionalAdded) {
     if (additionalAdded) fl.append(",");
     fl.append(field);

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

@@ -25,7 +25,9 @@
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.handler.component.ResponseBuilder;
 import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.request.SolrRequestInfo;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -63,6 +65,40 @@ public QParser createParser(
 
   private static class ReRankQParser extends QParser {
 
+    private boolean isExplainResults() {
+      final SolrRequestInfo ri = SolrRequestInfo.getRequestInfo();
+      if (null != ri) {
+        final ResponseBuilder rb = ri.getResponseBuilder();
+        if (null != rb) {
+          return rb.isDebugResults();
+        }
+      }
+
+      // HACK: The code below should not be used. It is preserved for backcompat
+      // on the slim remote chance that someone is using ReRankQParserPlugin
+      // w/o using SearchHandler+ResponseBuilder
+      //
+      // (It's also wrong, and doesn't account for thigns like debug=true
+      // or debug=all ... but as stated: it's for esoteric backcompat purposes
+      // only, so we're not going to change it and start returning "true"
+      // if existing code doesn't expect it
+
+      boolean debugQuery = params.getBool(CommonParams.DEBUG_QUERY, false);
+
+      if (!debugQuery) {
+        String[] debugParams = params.getParams(CommonParams.DEBUG);
+        if (debugParams != null) {
+          for (String debugParam : debugParams) {
+            if ("true".equals(debugParam)) {
+              debugQuery = true;
+              break;
+            }
+          }
+        }
+      }
+      return debugQuery;
+    }
+
     public ReRankQParser(
         String query, SolrParams localParams, SolrParams params, SolrQueryRequest req) {
       super(query, localParams, params, req);
@@ -88,19 +124,8 @@ public Query parse() throws SyntaxError {
 
       String mainScale = localParams.get(RERANK_MAIN_SCALE);
       String reRankScale = localParams.get(RERANK_SCALE);
-      boolean debugQuery = params.getBool(CommonParams.DEBUG_QUERY, false);
 
-      if (!debugQuery) {
-        String[] debugParams = params.getParams(CommonParams.DEBUG);
-        if (debugParams != null) {
-          for (String debugParam : debugParams) {
-            if ("true".equals(debugParam)) {
-              debugQuery = true;
-              break;
-            }
-          }
-        }
-      }
+      final boolean explainResults = isExplainResults();
 
       double reRankScaleWeight = reRankWeight;
 
@@ -111,15 +136,15 @@ public Query parse() throws SyntaxError {
               reRankScaleWeight,
               reRankOperator,
               new ReRankQueryRescorer(reRankQuery, 1, ReRankOperator.REPLACE),
-              debugQuery);
+              explainResults);
 
       if (reRankScaler.scaleScores()) {
         // Scaler applies the weighting instead of the rescorer
         reRankWeight = 1;
       }
 
       return new ReRankQuery(
-          reRankQuery, reRankDocs, reRankWeight, reRankOperator, reRankScaler, debugQuery);
+          reRankQuery, reRankDocs, reRankWeight, reRankOperator, reRankScaler, explainResults);
     }
   }
 
@@ -165,7 +190,7 @@ protected float combine(
   private static final class ReRankQuery extends AbstractReRankQuery {
     private final Query reRankQuery;
     private final double reRankWeight;
-    private final boolean debugQuery;
+    private final boolean explainResults;
 
     @Override
     public int hashCode() {
@@ -198,7 +223,7 @@ public ReRankQuery(
         double reRankWeight,
         ReRankOperator reRankOperator,
         ReRankScaler reRankScaler,
-        boolean debugQuery) {
+        boolean explainResults) {
       super(
           defaultQuery,
           reRankDocs,
@@ -207,7 +232,7 @@ public ReRankQuery(
           reRankOperator);
       this.reRankQuery = reRankQuery;
       this.reRankWeight = reRankWeight;
-      this.debugQuery = debugQuery;
+      this.explainResults = explainResults;
     }
 
     @Override
@@ -246,13 +271,13 @@ public String toString(String s) {
     @Override
     protected Query rewrite(Query rewrittenMainQuery) throws IOException {
       return new ReRankQuery(
-              reRankQuery, reRankDocs, reRankWeight, reRankOperator, reRankScaler, debugQuery)
+              reRankQuery, reRankDocs, reRankWeight, reRankOperator, reRankScaler, explainResults)
           .wrap(rewrittenMainQuery);
     }
 
     @Override
     public boolean getCache() {
-      if (reRankScaler.scaleScores() && debugQuery) {
+      if (reRankScaler.scaleScores() && explainResults) {
         // Caching breaks explain when reRankScaling is used.
         return false;
       } else {

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

@@ -32,7 +32,7 @@ public class ReRankScaler {
   protected int mainQueryMax = -1;
   protected int reRankQueryMin = -1;
   protected int reRankQueryMax = -1;
-  protected boolean debugQuery;
+  protected boolean explainResults;
   protected ReRankOperator reRankOperator;
   protected ReRankScalerExplain reRankScalerExplain;
   private QueryRescorer replaceRescorer;
@@ -45,11 +45,11 @@ public ReRankScaler(
       double reRankScaleWeight,
       ReRankOperator reRankOperator,
       QueryRescorer replaceRescorer,
-      boolean debugQuery)
+      boolean explainResults)
       throws SyntaxError {
 
     this.reRankScaleWeight = reRankScaleWeight;
-    this.debugQuery = debugQuery;
+    this.explainResults = explainResults;
     this.reRankScalerExplain = new ReRankScalerExplain(mainScale, reRankScale);
     this.replaceRescorer = replaceRescorer;
     if (reRankOperator != ReRankOperator.ADD
@@ -171,12 +171,12 @@ public ScoreDoc[] scaleScores(ScoreDoc[] originalDocs, ScoreDoc[] rescoredDocs,
       scaledOriginalScoreMap = originalScoreMap;
     }
 
-    this.reRankSet = debugQuery ? new HashSet<>() : null;
+    this.reRankSet = explainResults ? new HashSet<>() : null;
 
     for (int i = 0; i < howMany; i++) {
       ScoreDoc rescoredDoc = rescoredDocs[i];
       int doc = rescoredDoc.doc;
-      if (debugQuery) {
+      if (explainResults) {
         reRankSet.add(doc);
       }
       float score = rescoredDoc.score;
@@ -345,7 +345,14 @@ public Explanation explain(
       int doc, Explanation mainQueryExplain, Explanation reRankQueryExplain) {
     float reRankScore = reRankQueryExplain.getDetails()[1].getValue().floatValue();
     float mainScore = mainQueryExplain.getValue().floatValue();
-    if (reRankSet.contains(doc)) {
+    if (null == reRankSet) {
+      // we don't have the data needed to accurately report scaling,
+      // probably due to distributed request
+      return Explanation.match(
+          reRankScore,
+          "ReRank Scaling effects unkown, consider using distrib.singlePass=true (see https://issues.apache.org/jira/browse/SOLR-17299)",
+          reRankQueryExplain);
+    } else if (reRankSet.contains(doc)) {
       if (scaleMainScores() && scaleReRankScores()) {
         if (reRankScore > 0) {
           MinMaxExplain mainScaleExplain = reRankScalerExplain.getMainScaleExplain();