SOLR-13877: fix NPE in expand component

* This could happen when expand component is not used with collapse
  and matched docs have fewer unique values
This commit is contained in:
Munendra S N 2019-10-29 13:45:53 +05:30
parent 7d0dbdaf62
commit 2055983d80
3 changed files with 40 additions and 33 deletions

View File

@ -62,6 +62,8 @@ Bug Fixes
* SOLR-12393: Compute score if requested even when expanded docs not sorted by score in ExpandComponent. * SOLR-12393: Compute score if requested even when expanded docs not sorted by score in ExpandComponent.
(David Smiley, Munendra S N) (David Smiley, Munendra S N)
* SOLR-13877: Fix NPE in expand component when matched docs have fewer unique values. (Munendra S N)
Other Changes Other Changes
--------------------- ---------------------

View File

@ -327,11 +327,7 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
} }
if(count > 0 && count < 200) { if(count > 0 && count < 200) {
try {
groupQuery = getGroupQuery(field, count, ordBytes); groupQuery = getGroupQuery(field, count, ordBytes);
} catch(Exception e) {
throw new IOException(e);
}
} }
} else { } else {
groupSet = new LongHashSet(docList.size()); groupSet = new LongHashSet(docList.size());
@ -689,16 +685,15 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
int size, int size,
LongHashSet groupSet) { LongHashSet groupSet) {
BytesRef[] bytesRefs = new BytesRef[size]; List<BytesRef> bytesRefs = new ArrayList<>(size);
BytesRefBuilder term = new BytesRefBuilder(); BytesRefBuilder term = new BytesRefBuilder();
Iterator<LongCursor> it = groupSet.iterator(); Iterator<LongCursor> it = groupSet.iterator();
int index = -1;
while (it.hasNext()) { while (it.hasNext()) {
LongCursor cursor = it.next(); LongCursor cursor = it.next();
String stringVal = numericToString(ft, cursor.value); String stringVal = numericToString(ft, cursor.value);
ft.readableToIndexed(stringVal, term); ft.readableToIndexed(stringVal, term);
bytesRefs[++index] = term.toBytesRef(); bytesRefs.add(term.toBytesRef());
} }
return new TermInSetQuery(fname, bytesRefs); return new TermInSetQuery(fname, bytesRefs);
@ -738,13 +733,12 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
private Query getGroupQuery(String fname, private Query getGroupQuery(String fname,
int size, int size,
IntObjectHashMap<BytesRef> ordBytes) throws Exception { IntObjectHashMap<BytesRef> ordBytes) {
BytesRef[] bytesRefs = new BytesRef[size]; List<BytesRef> bytesRefs = new ArrayList<>(size);
int index = -1;
Iterator<IntObjectCursor<BytesRef>>it = ordBytes.iterator(); Iterator<IntObjectCursor<BytesRef>>it = ordBytes.iterator();
while (it.hasNext()) { while (it.hasNext()) {
IntObjectCursor<BytesRef> cursor = it.next(); IntObjectCursor<BytesRef> cursor = it.next();
bytesRefs[++index] = cursor.value; bytesRefs.add(cursor.value);
} }
return new TermInSetQuery(fname, bytesRefs); return new TermInSetQuery(fname, bytesRefs);
} }

View File

@ -91,17 +91,7 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
{"id","7", "term_s", "YYYY", group, "1"+floatAppend, "test_i", "1", "test_l", "100000", "test_f", "2000", "type_s", "child"}, {"id","7", "term_s", "YYYY", group, "1"+floatAppend, "test_i", "1", "test_l", "100000", "test_f", "2000", "type_s", "child"},
{"id","8", "term_s","YYYY", group, "2"+floatAppend, "test_i", "2", "test_l", "100000", "test_f", "200", "type_s", "child"} {"id","8", "term_s","YYYY", group, "2"+floatAppend, "test_i", "2", "test_l", "100000", "test_f", "200", "type_s", "child"}
}; };
// randomize addition of docs into bunch of segments createIndex(docs);
// TODO there ought to be a test utility to do this; even add in batches
Collections.shuffle(Arrays.asList(docs), random());
for (String[] doc : docs) {
assertU(adoc(doc));
if (random().nextBoolean()) {
assertU(commit());
}
}
assertU(commit());
ModifiableSolrParams params = new ModifiableSolrParams(); ModifiableSolrParams params = new ModifiableSolrParams();
params.add("q", "*:*"); params.add("q", "*:*");
@ -165,7 +155,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
//Test override expand.q //Test override expand.q
params = new ModifiableSolrParams(); params = new ModifiableSolrParams();
params.add("q", "type_s:parent"); params.add("q", "type_s:parent");
params.add("defType", "edismax"); params.add("defType", "edismax");
@ -186,7 +175,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
//Test override expand.fq //Test override expand.fq
params = new ModifiableSolrParams(); params = new ModifiableSolrParams();
params.add("q", "*:*"); params.add("q", "*:*");
params.add("fq", "type_s:parent"); params.add("fq", "type_s:parent");
@ -207,7 +195,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
); );
//Test override expand.fq and expand.q //Test override expand.fq and expand.q
params = new ModifiableSolrParams(); params = new ModifiableSolrParams();
params.add("q", "*:*"); params.add("q", "*:*");
params.add("fq", "type_s:parent"); params.add("fq", "type_s:parent");
@ -229,7 +216,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
); );
//Test expand.rows //Test expand.rows
params = new ModifiableSolrParams(); params = new ModifiableSolrParams();
params.add("q", "*:*"); params.add("q", "*:*");
params.add("fq", "{!collapse field="+group+hint+"}"); params.add("fq", "{!collapse field="+group+hint+"}");
@ -250,7 +236,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
//Test no group results //Test no group results
params = new ModifiableSolrParams(); params = new ModifiableSolrParams();
params.add("q", "test_i:5"); params.add("q", "test_i:5");
params.add("fq", "{!collapse field="+group+hint+"}"); params.add("fq", "{!collapse field="+group+hint+"}");
@ -264,7 +249,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
); );
//Test zero results //Test zero results
params = new ModifiableSolrParams(); params = new ModifiableSolrParams();
params.add("q", "test_i:5532535"); params.add("q", "test_i:5532535");
params.add("fq", "{!collapse field="+group+hint+"}"); params.add("fq", "{!collapse field="+group+hint+"}");
@ -278,7 +262,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
); );
//Test key-only fl //Test key-only fl
params = new ModifiableSolrParams(); params = new ModifiableSolrParams();
params.add("q", "*:*"); params.add("q", "*:*");
params.add("fq", "{!collapse field="+group+hint+"}"); params.add("fq", "{!collapse field="+group+hint+"}");
@ -299,7 +282,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
); );
//Test key-only fl with score but no sorting //Test key-only fl with score but no sorting
assertQ(req(params, "fl", "id,score"), "*[count(/response/result/doc)=2]", assertQ(req(params, "fl", "id,score"), "*[count(/response/result/doc)=2]",
"*[count(/response/lst[@name='expanded']/result)=2]", "*[count(/response/lst[@name='expanded']/result)=2]",
"/response/result/doc[1]/str[@name='id'][.='2']", "/response/result/doc[1]/str[@name='id'][.='2']",
@ -326,7 +308,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
); );
//Test fl with score, sort by non-score //Test fl with score, sort by non-score
assertQ(req(params, "expand.sort", "test_l desc", "fl", "id,test_i,score"), assertQ(req(params, "expand.sort", "test_l desc", "fl", "id,test_i,score"),
"*[count(/response/result/doc)=2]", "*[count(/response/result/doc)=2]",
"count(/response/lst[@name='expanded']/result)=2", "count(/response/lst[@name='expanded']/result)=2",
@ -342,7 +323,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
); );
//Test fl with score with multi-sort //Test fl with score with multi-sort
assertQ(req(params, "expand.sort", "test_l desc, score asc", "fl", "id,test_i,score"), assertQ(req(params, "expand.sort", "test_l desc, score asc", "fl", "id,test_i,score"),
"*[count(/response/result/doc)=2]", "*[count(/response/result/doc)=2]",
"count(/response/lst[@name='expanded']/result)=2", "count(/response/lst[@name='expanded']/result)=2",
@ -356,6 +336,22 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
"count(//*[@name='score' and .='NaN'])=0", "count(//*[@name='score' and .='NaN'])=0",
"count(/response/lst[@name='expanded']/result/doc[number(*/@name='score')!=number(*/@name='test_i')])=0" "count(/response/lst[@name='expanded']/result/doc[number(*/@name='score')!=number(*/@name='test_i')])=0"
); );
// Test for expand with collapse
// when matched docs have fewer unique values
params = params("q", "*:*", "sort", "id asc", "fl", "id", "rows", "6", "expand", "true", "expand.sort", "id asc");
assertQ(req(params, "expand.field", "term_s"),
"*[count(/response/result/doc)=6]",
"/response/lst[@name='expanded']/result[@name='YYYY']/doc[1]/str[@name='id'][.='7']",
"/response/lst[@name='expanded']/result[@name='YYYY']/doc[2]/str[@name='id'][.='8']",
"count(//*[@name='score'])=0"
);
assertQ(req(params, "expand.field", "test_f"),
"*[count(/response/result/doc)=6]",
"/response/lst[@name='expanded']/result[@name='200.0']/doc[1]/str[@name='id'][.='8']",
"/response/lst[@name='expanded']/result[@name='2000.0']/doc[1]/str[@name='id'][.='7']",
"count(//*[@name='score'])=0"
);
} }
@Test @Test
@ -416,4 +412,19 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
resetExceptionIgnores(); resetExceptionIgnores();
} }
/**
* randomize addition of docs into bunch of segments
* TODO: there ought to be a test utility to do this; even add in batches
*/
private void createIndex(String[][] docs) {
Collections.shuffle(Arrays.asList(docs), random());
for (String[] doc : docs) {
assertU(adoc(doc));
if (random().nextBoolean()) {
assertU(commit());
}
}
assertU(commit());
}
} }