You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by jb...@apache.org on 2020/03/09 01:12:35 UTC
[lucene-solr] branch branch_8x updated: SOLR-14073: Fix segment
look ahead NPE in CollapsingQParserPlugin
This is an automated email from the ASF dual-hosted git repository.
jbernste pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new 904407f SOLR-14073: Fix segment look ahead NPE in CollapsingQParserPlugin
904407f is described below
commit 904407f0b640ef5135c3940aec80225d7aee7165
Author: Joel Bernstein <jb...@apache.org>
AuthorDate: Fri Mar 6 15:03:25 2020 -0500
SOLR-14073: Fix segment look ahead NPE in CollapsingQParserPlugin
---
.../solr/search/CollapsingQParserPlugin.java | 29 ++++++++--
.../solr/search/TestCollapseQParserPlugin.java | 63 ++++++++++++++++++++++
2 files changed, 88 insertions(+), 4 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
index 3630ddc..690f806 100644
--- a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
+++ b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
@@ -526,9 +526,15 @@ public class CollapsingQParserPlugin extends QParserPlugin {
int segments,
DocValuesProducer collapseValuesProducer,
int nullPolicy,
- IntIntHashMap boostDocsMap) throws IOException {
+ IntIntHashMap boostDocsMap,
+ IndexSearcher searcher) throws IOException {
this.maxDoc = maxDoc;
this.contexts = new LeafReaderContext[segments];
+ List<LeafReaderContext> con = searcher.getTopReaderContext().leaves();
+ for(int i=0; i<con.size(); i++) {
+ contexts[i] = con.get(i);
+ }
+
this.collapsedSet = new FixedBitSet(maxDoc);
this.collapseValuesProducer = collapseValuesProducer;
this.collapseValues = collapseValuesProducer.getSorted(null);
@@ -756,9 +762,15 @@ public class CollapsingQParserPlugin extends QParserPlugin {
int nullPolicy,
int size,
String field,
- IntIntHashMap boostDocsMap) {
+ IntIntHashMap boostDocsMap,
+ IndexSearcher searcher) {
this.maxDoc = maxDoc;
this.contexts = new LeafReaderContext[segments];
+ List<LeafReaderContext> con = searcher.getTopReaderContext().leaves();
+ for(int i=0; i<con.size(); i++) {
+ contexts[i] = con.get(i);
+ }
+
this.collapsedSet = new FixedBitSet(maxDoc);
this.nullValue = nullValue;
this.nullPolicy = nullPolicy;
@@ -928,6 +940,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
*/
private static class OrdFieldValueCollector extends DelegatingCollector {
private LeafReaderContext[] contexts;
+
private DocValuesProducer collapseValuesProducer;
private SortedDocValues collapseValues;
protected OrdinalMap ordinalMap;
@@ -958,6 +971,10 @@ public class CollapsingQParserPlugin extends QParserPlugin {
this.maxDoc = maxDoc;
this.contexts = new LeafReaderContext[segments];
+ List<LeafReaderContext> con = searcher.getTopReaderContext().leaves();
+ for(int i=0; i<con.size(); i++) {
+ contexts[i] = con.get(i);
+ }
this.collapseValuesProducer = collapseValuesProducer;
this.collapseValues = collapseValuesProducer.getSorted(null);
if(collapseValues instanceof MultiDocValues.MultiSortedDocValues) {
@@ -1151,6 +1168,10 @@ public class CollapsingQParserPlugin extends QParserPlugin {
this.maxDoc = maxDoc;
this.contexts = new LeafReaderContext[segments];
+ List<LeafReaderContext> con = searcher.getTopReaderContext().leaves();
+ for(int i=0; i<con.size(); i++) {
+ contexts[i] = con.get(i);
+ }
this.collapseField = collapseField;
this.nullValue = nullValue;
this.nullPolicy = nullPolicy;
@@ -1346,7 +1367,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
if (collapseFieldType instanceof StrField) {
- return new OrdScoreCollector(maxDoc, leafCount, docValuesProducer, nullPolicy, boostDocs);
+ return new OrdScoreCollector(maxDoc, leafCount, docValuesProducer, nullPolicy, boostDocs, searcher);
} else if (isNumericCollapsible(collapseFieldType)) {
@@ -1365,7 +1386,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
}
}
- return new IntScoreCollector(maxDoc, leafCount, nullValue, nullPolicy, size, collapseField, boostDocs);
+ return new IntScoreCollector(maxDoc, leafCount, nullValue, nullPolicy, size, collapseField, boostDocs, searcher);
} else {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
diff --git a/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java b/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
index e7dd357..09444e1 100644
--- a/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
+++ b/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
@@ -343,6 +343,69 @@ public class TestCollapseQParserPlugin extends SolrTestCaseJ4 {
}
}
+ @Test
+ public void testDoubleCollapse() {
+ testDoubleCollapse("group_s", "");
+ testDoubleCollapse("group_i", "");
+ }
+
+
+ /*
+ * SOLR-14073
+ * The double collapse causes a look ahead in the second collapse to a segment that was not visited by
+ * the by finally method of the first collapse. This specific test is meant to confirm that any feature
+ * that causes searches to not visit each segment (such as early query termination) doesn't break collapse.
+ */
+ private void testDoubleCollapse(String group, String hint) {
+ String[] doc = {"id","1", "term_s", "YYYY", group, "1", "test_i", "5", "test_l", "10", "test_f", "2000"};
+ assertU(adoc(doc));
+ assertU(commit());
+ String[] doc1 = {"id","2", "term_s","YYYY", group, "2", "test_i", "50", "test_l", "100", "test_f", "200"};
+ assertU(adoc(doc1));
+
+
+
+ String[] doc2 = {"id","3", "term_s", "YYYY", "test_i", "5000", "test_l", "100", "test_f", "200"};
+ assertU(adoc(doc2));
+ assertU(commit());
+ String[] doc3 = {"id","4", "term_s", "YYYY", "test_i", "500", "test_l", "1000", "test_f", "2000"};
+ assertU(adoc(doc3));
+
+
+ String[] doc4 = {"id","5", "term_s", "YYYN", group, "2", "test_i", "4", "test_l", "10", "test_f", "2000"};
+ assertU(adoc(doc4));
+ assertU(commit());
+ String[] doc5 = {"id","6", "term_s","YYYY", group, "2", "test_i", "10", "test_l", "100", "test_f", "200"};
+ assertU(adoc(doc5));
+ assertU(commit());
+
+ String[] doc6 = {"id","7", "term_s", "YYYY", group, "1", "test_i", "8", "test_l", "50", "test_f", "300"};
+ assertU(adoc(doc6));
+ assertU(commit());
+
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.add("q", "id:(1 2 5)");
+ params.add("fq", "{!collapse cost=200 field=term_s "+hint+"}");
+ params.add("fq", "{!collapse cost=400 field="+group+""+hint+"}");
+
+ params.add("defType", "edismax");
+ params.add("bf", "field(test_i)");
+ assertQ(req(params, "indent", "on"), "*[count(//doc)=1]",
+ "//result/doc[1]/str[@name='id'][.='2']"
+ );
+
+ params = new ModifiableSolrParams();
+ params.add("q", "id:(1 2 5)");
+ params.add("fq", "{!collapse cost=200 max=test_i field=term_s "+hint+"}");
+ params.add("fq", "{!collapse cost=400 max=test_i field="+group+""+hint+"}");
+
+ params.add("defType", "edismax");
+ params.add("bf", "field(test_i)");
+ assertQ(req(params, "indent", "on"), "*[count(//doc)=1]",
+ "//result/doc[1]/str[@name='id'][.='2']"
+ );
+ }
+
private void testCollapseQueries(String group, String hint, boolean numeric) throws Exception {