This is an automated email from the ASF dual-hosted git repository.
houston pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new ce64685c7f9 SOLR-9775: Fix NPEs in QueryResultKey
ce64685c7f9 is described below
commit ce64685c7f97c18fe7caadbf6ba39afc295f4806
Author: Houston Putman <[email protected]>
AuthorDate: Thu Apr 6 10:56:28 2023 -0400
SOLR-9775: Fix NPEs in QueryResultKey
---
solr/CHANGES.txt | 2 +
.../org/apache/solr/search/QueryResultKey.java | 33 ++++++++-----
.../org/apache/solr/core/QueryResultKeyTest.java | 57 ++++++++++++++++++++++
3 files changed, 80 insertions(+), 12 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index e6992a6f038..aaf7d865987 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -68,6 +68,8 @@ Bug Fixes
* SOLR-16728: Fix Classloading Exception for inter-node requests when using
SSL and HTTP2.
All Jetty classes are able to be shared between the Jetty server and webApp
now. (Houston Putman)
+* NPE in QueryResultKey when running clustering search query in some cases.
(Roman Kagan, Christine Poerschke via Eric Pugh)
+
Dependency Upgrades
---------------------
* PR#1494: Upgrade forbiddenapis to 3.5 (Uwe Schindler)
diff --git a/solr/core/src/java/org/apache/solr/search/QueryResultKey.java
b/solr/core/src/java/org/apache/solr/search/QueryResultKey.java
index 6b68a13e581..caeeb337895 100644
--- a/solr/core/src/java/org/apache/solr/search/QueryResultKey.java
+++ b/solr/core/src/java/org/apache/solr/search/QueryResultKey.java
@@ -17,7 +17,10 @@
package org.apache.solr.search;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.Sort;
import org.apache.lucene.search.SortField;
@@ -33,7 +36,7 @@ public final class QueryResultKey implements Accountable {
final Query query;
final Sort sort;
- final SortField[] sfields;
+ final List<SortField> sfields;
final List<Query> filters;
final int nc_flags; // non-comparable flags... ignored by hashCode and equals
final int minExactCount;
@@ -41,8 +44,6 @@ public final class QueryResultKey implements Accountable {
private final int hc; // cached hashCode
private final long ramBytesUsed; // cached
- private static SortField[] defaultSort = new SortField[0];
-
public QueryResultKey(Query query, List<Query> filters, Sort sort, int
nc_flags) {
this(query, filters, sort, nc_flags, Integer.MAX_VALUE);
}
@@ -51,20 +52,28 @@ public final class QueryResultKey implements Accountable {
Query query, List<Query> filters, Sort sort, int nc_flags, int
minExactCount) {
this.query = query;
this.sort = sort;
- this.filters = filters;
this.nc_flags = nc_flags;
this.minExactCount = minExactCount;
int h = query.hashCode();
- if (filters != null) {
- for (Query filt : filters)
+ if (filters == null) {
+ this.filters = null;
+ } else {
+ this.filters =
filters.stream().filter(Objects::nonNull).collect(Collectors.toList());
+ for (Query filt : this.filters) {
// NOTE: simple summation used here so keys with the same filters but
in
// different orders get the same hashCode
h += filt.hashCode();
+ }
}
- sfields = (this.sort != null) ? this.sort.getSort() : defaultSort;
+ if (this.sort == null) {
+ this.sfields = List.of();
+ } else {
+ this.sfields =
+
Arrays.stream(sort.getSort()).filter(Objects::nonNull).collect(Collectors.toList());
+ }
long ramSfields = RamUsageEstimator.NUM_BYTES_ARRAY_HEADER;
for (SortField sf : sfields) {
h = h * 29 + sf.hashCode();
@@ -79,7 +88,7 @@ public final class QueryResultKey implements Accountable {
+ ramSfields
+ RamUsageEstimator.sizeOfObject(query,
RamUsageEstimator.QUERY_DEFAULT_RAM_BYTES_USED)
+ RamUsageEstimator.sizeOfObject(
- filters, RamUsageEstimator.QUERY_DEFAULT_RAM_BYTES_USED);
+ this.filters, RamUsageEstimator.QUERY_DEFAULT_RAM_BYTES_USED);
}
@Override
@@ -100,14 +109,14 @@ public final class QueryResultKey implements Accountable {
// check for the thing most likely to be different (and the fastest things)
// first.
- if (this.sfields.length != other.sfields.length) return false;
+ if (this.sfields.size() != other.sfields.size()) return false;
if (!this.query.equals(other.query)) return false;
if (!unorderedCompare(this.filters, other.filters)) return false;
if (this.minExactCount != other.minExactCount) return false;
- for (int i = 0; i < sfields.length; i++) {
- SortField sf1 = this.sfields[i];
- SortField sf2 = other.sfields[i];
+ for (int i = 0; i < sfields.size(); i++) {
+ SortField sf1 = this.sfields.get(i);
+ SortField sf2 = other.sfields.get(i);
if (!sf1.equals(sf2)) return false;
}
diff --git a/solr/core/src/test/org/apache/solr/core/QueryResultKeyTest.java
b/solr/core/src/test/org/apache/solr/core/QueryResultKeyTest.java
index e5f53892b24..d663b6db600 100644
--- a/solr/core/src/test/org/apache/solr/core/QueryResultKeyTest.java
+++ b/solr/core/src/test/org/apache/solr/core/QueryResultKeyTest.java
@@ -108,6 +108,63 @@ public class QueryResultKeyTest extends SolrTestCaseJ4 {
assertKeyNotEquals(key1, key2);
}
+ public void testCreateKeyWithNullFilterList() {
+ Sort sort = new Sort(new SortField("test", SortField.Type.INT));
+ BooleanQuery.Builder query = new BooleanQuery.Builder();
+ query.add(new TermQuery(new Term("test", "field")), Occur.MUST);
+
+ QueryResultKey qrk1 = new QueryResultKey(query.build(), null, sort, 1);
+ assertNotNull(qrk1);
+ }
+
+ public void testCreateKeyWithNullFilterListAndNullSort() {
+ BooleanQuery.Builder query = new BooleanQuery.Builder();
+ query.add(new TermQuery(new Term("test", "field")), Occur.MUST);
+
+ QueryResultKey qrk1 = new QueryResultKey(query.build(), null, null, 1);
+ assertNotNull(qrk1);
+ }
+
+ public void testNullFilterInFiltersList() {
+ // the hashcode should be the same even when the list
+ // of filters has a null filter
+
+ Sort sort = new Sort(new SortField("test", SortField.Type.INT));
+ BooleanQuery.Builder query = new BooleanQuery.Builder();
+ query.add(new TermQuery(new Term("test", "field")), Occur.MUST);
+
+ List<Query> filters =
+ Arrays.asList(
+ new TermQuery(new Term("test", "field")),
+ new TermQuery(new Term("test2", "field2")),
+ null);
+ QueryResultKey qrk1 = new QueryResultKey(query.build(), filters, sort, 1);
+
+ List<Query> filters2 =
+ Arrays.asList(
+ new TermQuery(new Term("test2", "field2")), new TermQuery(new
Term("test", "field")));
+ QueryResultKey qrk2 = new QueryResultKey(query.build(), filters2, sort, 1);
+ assertKeyEquals(qrk1, qrk2);
+ }
+
+ public void testNullSortFieldInSort() {
+ // the hashcode should be the same even when the list
+ // of SortFields has a null value.
+
+ Sort sort1 = new Sort(new SortField("test", SortField.Type.INT), null);
+ Sort sort2 = new Sort(new SortField("test", SortField.Type.INT), null);
+ BooleanQuery.Builder query = new BooleanQuery.Builder();
+ query.add(new TermQuery(new Term("test", "field")), Occur.MUST);
+
+ List<Query> filters =
+ Arrays.asList(
+ new TermQuery(new Term("test", "field")), new TermQuery(new
Term("test2", "field2")));
+ QueryResultKey qrk1 = new QueryResultKey(query.build(), filters, sort1, 1);
+
+ QueryResultKey qrk2 = new QueryResultKey(query.build(), filters, sort2, 1);
+ assertKeyEquals(qrk1, qrk2);
+ }
+
public void testRandomQueryKeyEquality() {
final int minIters = atLeast(100 * 1000);
