This is an automated email from the ASF dual-hosted git repository.
uschindler pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new a72c993 SOLR-14758: Fix NPE in QueryComponent.mergeIds when using
timeAllowed and sorting (#251)
a72c993 is described below
commit a72c99312380ef88fedd57d5f9806dc0f48406a1
Author: Uwe Schindler <[email protected]>
AuthorDate: Tue Aug 10 18:27:41 2021 +0200
SOLR-14758: Fix NPE in QueryComponent.mergeIds when using timeAllowed and
sorting (#251)
Signed-off-by: Uwe Schindler <[email protected]>
Co-authored-by: Bram Van Dam <[email protected]>
---
solr/CHANGES.txt | 3 +++
.../org/apache/solr/handler/component/QueryComponent.java | 2 +-
.../component/DistributedQueryComponentCustomSortTest.java | 9 +++++++++
.../org/apache/solr/BaseDistributedSearchTestCase.java | 14 +++++++++-----
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a1b3526..326f91b 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -412,6 +412,9 @@ Bug Fixes
* SOLR-15575: Propagate request level basic auth creds from the top-level
async CollectionAdminRequest to internally
used async requests, such as async status checking (Timothy Potter)
+* SOLR-14758: Fix NPE in QueryComponent.mergeIds when using timeAllowed and
sorting
+ (Bram Van Dam, Uwe Schindler)
+
Other Changes
---------------------
* SOLR-15566: Clarify ref guide documentation about SQL queries with `SELECT
*` requiring a `LIMIT` clause (Timothy Potter)
diff --git
a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
index 5b85143..d89427a 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
@@ -955,7 +955,7 @@ public class QueryComponent extends SearchComponent
@SuppressWarnings("unchecked")
NamedList<List<Object>> sortFieldValues =
(NamedList<List<Object>>)(srsp.getSolrResponse().getResponse().get("sort_values"));
- if (sortFieldValues.size()==0 && // we bypass merging this response
only if it's partial itself
+ if ((null == sortFieldValues || sortFieldValues.size()==0) && // we
bypass merging this response only if it's partial itself
thisResponseIsPartial) { // but not the previous
one!!
continue; //fsv timeout yields empty sort_vlaues
}
diff --git
a/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentCustomSortTest.java
b/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentCustomSortTest.java
index 7efde0c..ca7e850 100644
---
a/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentCustomSortTest.java
+++
b/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentCustomSortTest.java
@@ -18,10 +18,12 @@ package org.apache.solr.handler.component;
import org.apache.solr.BaseDistributedSearchTestCase;
import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.response.SolrQueryResponse;
import org.junit.BeforeClass;
import org.junit.Test;
import java.nio.ByteBuffer;
+import java.util.Objects;
/**
* Test for QueryComponent's distributed querying
@@ -118,5 +120,12 @@ public class DistributedQueryComponentCustomSortTest
extends BaseDistributedSear
assertFieldValues(rsp.getResults(), id, "7", "1", "6", "15","14","4",
"2", "18","17","16","10", "12", "3", "5", "9", "8", "13", "11");
rsp = query("q", "*:*", "fl", "id", "sort", "payload desc, id_i asc",
"rows", "20");
assertFieldValues(rsp.getResults(), id, "11", "13", "8", "9", "5", "3",
"12", "10","16","17","18", "2", "4","14","15", "6", "1", "7");
+
+ // Regression check on timeAllowed in combination with sorting, SOLR-14758
+ // Should see either a complete result or a partial result, but never an
NPE
+ rsp = queryServer(createParams("q", "text:d", "fl", "id", "sort", "payload
desc", "rows", "20", "timeAllowed", "1"));
+ if (!Objects.equals(Boolean.TRUE,
rsp.getHeader().getBooleanArg(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY)))
{
+ assertFieldValues(rsp.getResults(), id, "11", "13", "12");
+ }
}
}
diff --git
a/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java
b/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java
index 6759af8..fe19dea 100644
---
a/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java
+++
b/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java
@@ -645,11 +645,7 @@ public abstract class BaseDistributedSearchTestCase
extends SolrTestCaseJ4 {
*/
protected QueryResponse query(boolean setDistribParams, Object[] q) throws
Exception {
- final ModifiableSolrParams params = new ModifiableSolrParams();
-
- for (int i = 0; i < q.length; i += 2) {
- params.add(q[i].toString(), q[i + 1].toString());
- }
+ final ModifiableSolrParams params = createParams(q);
return query(setDistribParams, params);
}
@@ -1224,4 +1220,12 @@ public abstract class BaseDistributedSearchTestCase
extends SolrTestCaseJ4 {
Files.createDirectories(jettyHome.toPath().resolve("cores").resolve("collection1"));
}
+ protected ModifiableSolrParams createParams(Object ... q) {
+ final ModifiableSolrParams params = new ModifiableSolrParams();
+
+ for (int i = 0; i < q.length; i += 2) {
+ params.add(q[i].toString(), q[i + 1].toString());
+ }
+ return params;
+ }
}
