diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 2991a614054..5f92bc7447d 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -289,6 +289,9 @@ Bug Fixes * SOLR-8422: When authentication enabled, requests fail if sent to a node that doesn't host the collection (noble) +* SOLR-8059: &debug=results for distributed search when distrib.singlePass (sometimes activated + automatically) could result in an NPE. (David Smiley, Markus Jelsma) + Other Changes ---------------------- diff --git a/solr/core/src/java/org/apache/solr/handler/component/DebugComponent.java b/solr/core/src/java/org/apache/solr/handler/component/DebugComponent.java index 41fecf95d59..21755e1b71f 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/DebugComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/DebugComponent.java @@ -24,6 +24,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -230,13 +231,7 @@ public class DebugComponent extends SearchComponent hasGetDebugResponses = true; if (rb.isDebugResults()) { NamedList sexplain = (NamedList)sdebug.get("explain"); - for (int i = 0; i < sexplain.size(); i++) { - String id = sexplain.getName(i); - // TODO: lookup won't work for non-string ids... String vs Float - ShardDoc sdoc = rb.resultIds.get(id); - int idx = sdoc.positionInResponse; - arr[idx] = new NamedList.NamedListEntry<>(id, sexplain.getVal(i)); - } + SolrPluginUtils.copyNamedListIntoArrayByDocPosInResponse(sexplain, rb.resultIds, arr); } } } @@ -295,7 +290,7 @@ public class DebugComponent extends SearchComponent return namedList; } - Object merge(Object source, Object dest, Set exclude) { + protected Object merge(Object source, Object dest, Set exclude) { if (source == null) return dest; if (dest == null) { if (source instanceof NamedList) { @@ -306,6 +301,10 @@ public class DebugComponent extends SearchComponent } else { if (dest instanceof Collection) { + // merge as Set + if (!(dest instanceof Set)) { + dest = new LinkedHashSet<>((Collection) dest); + } if (source instanceof Collection) { ((Collection)dest).addAll((Collection)source); } else { @@ -337,7 +336,7 @@ public class DebugComponent extends SearchComponent NamedList dl = (NamedList)dest; for (int i=0; i[] arr = new NamedList.NamedListEntry[rb.resultIds.size()]; + NamedList.NamedListEntry[] arr = new NamedList.NamedListEntry[rb.resultIds.size()]; // TODO: make a generic routine to do automatic merging of id keyed data for (ShardRequest sreq : rb.finished) { @@ -187,21 +187,12 @@ public class HighlightComponent extends SearchComponent implements PluginInfoIni continue; } NamedList hl = (NamedList)srsp.getSolrResponse().getResponse().get("highlighting"); - for (int i=0; i(id, hl.getVal(i)); - } + SolrPluginUtils.copyNamedListIntoArrayByDocPosInResponse(hl, rb.resultIds, arr); } } // remove nulls in case not all docs were able to be retrieved - rb.rsp.add("highlighting", SolrPluginUtils.removeNulls(arr, new SimpleOrderedMap())); + rb.rsp.add("highlighting", SolrPluginUtils.removeNulls(arr, new SimpleOrderedMap<>())); } } diff --git a/solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java b/solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java index 48e767c3cdb..7bf195dab06 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java +++ b/solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java @@ -251,6 +251,10 @@ public class ResponseBuilder return this.mergeStrategies; } + public RankQuery getRankQuery() { + return rankQuery; + } + public void setRankQuery(RankQuery rankQuery) { this.rankQuery = rankQuery; } @@ -428,7 +432,8 @@ public class ResponseBuilder return cmd; } - Query wrap(Query q) { + /** Calls {@link RankQuery#wrap(Query)} if there's a rank query, otherwise just returns the query. */ + public Query wrap(Query q) { if(this.rankQuery != null) { return this.rankQuery.wrap(q); } else { diff --git a/solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java b/solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java index 97a9b572747..53777591ad8 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java @@ -457,14 +457,7 @@ public class TermVectorComponent extends SearchComponent implements SolrCoreAwar } // UniqueKey data - for (int i=0; i < nl.size(); i++) { - String key = nl.getName(i); - ShardDoc sdoc = rb.resultIds.get(key); - if (sdoc != null) {// can be null when rb.onePassDistributedQuery - int idx = sdoc.positionInResponse; - arr[idx] = new NamedList.NamedListEntry<>(key, nl.getVal(i)); - } - } + SolrPluginUtils.copyNamedListIntoArrayByDocPosInResponse(nl, rb.resultIds, arr); } } // remove nulls in case not all docs were able to be retrieved diff --git a/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java b/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java index 1f1321df4e7..3a180fd1491 100644 --- a/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java +++ b/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java @@ -57,6 +57,7 @@ import org.apache.solr.core.RequestParams; import org.apache.solr.core.SolrCore; import org.apache.solr.handler.component.HighlightComponent; import org.apache.solr.handler.component.ResponseBuilder; +import org.apache.solr.handler.component.ShardDoc; import org.apache.solr.handler.component.ShardRequest; import org.apache.solr.highlight.SolrHighlighter; import org.apache.solr.parser.QueryParser; @@ -811,6 +812,25 @@ public class SolrPluginUtils { return dest; } + /** Copies the given {@code namedList} assumed to have doc uniqueKey keyed data into {@code destArr} + * at the position of the document in the response. destArr is assumed to be the same size as + * {@code resultIds} is. {@code resultIds} comes from {@link ResponseBuilder#resultIds}. If the doc key + * isn't in {@code resultIds} then it is ignored. + * Note: most likely you will call {@link #removeNulls(Map.Entry[], NamedList)} sometime after calling this. */ + public static void copyNamedListIntoArrayByDocPosInResponse(NamedList namedList, Map resultIds, + Map.Entry[] destArr) { + assert resultIds.size() == destArr.length; + for (int i = 0; i < namedList.size(); i++) { + String id = namedList.getName(i); + // TODO: lookup won't work for non-string ids... String vs Float + ShardDoc sdoc = resultIds.get(id); + if (sdoc != null) { // maybe null when rb.onePassDistributedQuery + int idx = sdoc.positionInResponse; + destArr[idx] = new NamedList.NamedListEntry<>(id, namedList.getVal(i)); + } + } + } + /** * A subclass of SolrQueryParser that supports aliasing fields for * constructing DisjunctionMaxQueries. diff --git a/solr/core/src/test/org/apache/solr/handler/component/DistributedDebugComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/DistributedDebugComponentTest.java index dc9f8a60462..63b131ee966 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/DistributedDebugComponentTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/DistributedDebugComponentTest.java @@ -346,11 +346,17 @@ public class DistributedDebugComponentTest extends SolrJettyTestBase { public void testCompareWithNonDistributedRequest() throws SolrServerException, IOException { SolrQuery query = new SolrQuery(); - query.setQuery("id:1"); - query.setFilterQueries("id:[0 TO 10]"); + query.setQuery("id:1 OR id:2"); + query.setFilterQueries("id:[0 TO 10]", "id:[0 TO 5]"); + query.setRows(1); + query.setSort("id", SolrQuery.ORDER.asc); // thus only return id:1 since rows 1 query.set("debug", "true"); query.set("distrib", "true"); - query.setFields("id", "text"); + query.setFields("id"); + if (random().nextBoolean()) { // can affect rb.onePassDistributedQuery + query.addField("text"); + } + query.set(ShardParams.DISTRIB_SINGLE_PASS, random().nextBoolean()); query.set("shards", shard1 + "," + shard2); QueryResponse distribResponse = collection1.query(query);