You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2015/11/16 23:42:25 UTC
svn commit: r1714701 - in /lucene/dev/trunk/solr: ./
core/src/java/org/apache/solr/search/ core/src/test/org/apache/solr/search/
Author: hossman
Date: Mon Nov 16 22:42:25 2015
New Revision: 1714701
URL: http://svn.apache.org/viewvc?rev=1714701&view=rev
Log:
SOLR-8295: Fix NPE in collapse QParser when collapse field is missing from all docs in a segment
Modified:
lucene/dev/trunk/solr/CHANGES.txt
lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRandomCollapseQParserPlugin.java
Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1714701&r1=1714700&r2=1714701&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Mon Nov 16 22:42:25 2015
@@ -394,6 +394,7 @@ Bug Fixes
* SOLR-8284: JSON Facet API - fix NPEs when short form "sort:index" or "sort:count"
are used. (Michael Sun via yonik)
+* SOLR-8295: Fix NPE in collapse QParser when collapse field is missing from all docs in a segment (hossman)
Optimizations
----------------------
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java?rev=1714701&r1=1714700&r2=1714701&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java Mon Nov 16 22:42:25 2015
@@ -822,7 +822,7 @@ public class CollapsingQParserPlugin ext
int currentContext = 0;
int currentDocBase = 0;
- collapseValues = contexts[currentContext].reader().getNumericDocValues(this.field);
+ collapseValues = DocValues.getNumeric(contexts[currentContext].reader(), this.field);
int nextDocBase = currentContext+1 < contexts.length ? contexts[currentContext+1].docBase : maxDoc;
leafDelegate = delegate.getLeafCollector(contexts[currentContext]);
DummyScorer dummy = new DummyScorer();
@@ -838,7 +838,7 @@ public class CollapsingQParserPlugin ext
nextDocBase = currentContext+1 < contexts.length ? contexts[currentContext+1].docBase : maxDoc;
leafDelegate = delegate.getLeafCollector(contexts[currentContext]);
leafDelegate.setScorer(dummy);
- collapseValues = contexts[currentContext].reader().getNumericDocValues(this.field);
+ collapseValues = DocValues.getNumeric(contexts[currentContext].reader(), this.field);
}
int contextDoc = globalDoc-currentDocBase;
@@ -1101,7 +1101,7 @@ public class CollapsingQParserPlugin ext
this.contexts[context.ord] = context;
this.docBase = context.docBase;
this.collapseStrategy.setNextReader(context);
- this.collapseValues = context.reader().getNumericDocValues(this.collapseField);
+ this.collapseValues = DocValues.getNumeric(context.reader(), this.collapseField);
}
public void collect(int contextDoc) throws IOException {
@@ -1117,7 +1117,7 @@ public class CollapsingQParserPlugin ext
int currentContext = 0;
int currentDocBase = 0;
- this.collapseValues = contexts[currentContext].reader().getNumericDocValues(this.collapseField);
+ this.collapseValues = DocValues.getNumeric(contexts[currentContext].reader(), this.collapseField);
int nextDocBase = currentContext+1 < contexts.length ? contexts[currentContext+1].docBase : maxDoc;
leafDelegate = delegate.getLeafCollector(contexts[currentContext]);
DummyScorer dummy = new DummyScorer();
@@ -1140,7 +1140,7 @@ public class CollapsingQParserPlugin ext
nextDocBase = currentContext+1 < contexts.length ? contexts[currentContext+1].docBase : maxDoc;
leafDelegate = delegate.getLeafCollector(contexts[currentContext]);
leafDelegate.setScorer(dummy);
- this.collapseValues = contexts[currentContext].reader().getNumericDocValues(this.collapseField);
+ this.collapseValues = DocValues.getNumeric(contexts[currentContext].reader(), this.collapseField);
}
int contextDoc = globalDoc-currentDocBase;
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java?rev=1714701&r1=1714700&r2=1714701&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java Mon Nov 16 22:42:25 2015
@@ -653,6 +653,26 @@ public class TestCollapseQParserPlugin e
"//result/doc[1]/float[@name='id'][.='5.0']",
"//result/doc[2]/float[@name='id'][.='1.0']");
+ // Test collapse using selector field in no docs
+ // tie selector in all of these cases, so index order applies
+ for (String selector : new String[] {
+ " min=bogus_ti ", " sort='bogus_ti asc' ",
+ " max=bogus_ti ", " sort='bogus_ti desc' ",
+ " min=bogus_tf ", " sort='bogus_tf asc' ",
+ " max=bogus_tf ", " sort='bogus_tf desc' ",
+ " sort='bogus_td asc' ", " sort='bogus_td desc' ",
+ " sort='bogus_s asc' ", " sort='bogus_s desc' ",
+ }) {
+ params = new ModifiableSolrParams();
+ params.add("q", "*:*");
+ params.add("fq", "{!collapse field="+group + selector + hint+"}");
+ params.add("sort", "id asc");
+ assertQ(req(params),
+ "*[count(//doc)=2]",
+ "//result/doc[1]/float[@name='id'][.='1.0']",
+ "//result/doc[2]/float[@name='id'][.='5.0']");
+ }
+
// attempting to use cscore() in sort local param should fail
assertQEx("expected error trying to sort on a function that includes cscore()",
req(params("q", "{!func}sub(sub(test_tl,1000),id)",
@@ -773,6 +793,74 @@ public class TestCollapseQParserPlugin e
assertQ(req(params), "*[count(//doc)=0]");
}
+ public void testNoDocsHaveGroupField() throws Exception {
+ // as unlikely as this test seems, it's important for the possibility that a segment exists w/o
+ // any live docs that have DocValues for the group field -- ie: every doc in segment is in null group.
+
+ assertU(adoc("id", "1", "group_s", "group1", "test_ti", "5", "test_tl", "10"));
+ assertU(commit());
+ assertU(adoc("id", "2", "group_s", "group1", "test_ti", "5", "test_tl", "1000"));
+ assertU(adoc("id", "3", "group_s", "group1", "test_ti", "5", "test_tl", "1000"));
+ assertU(adoc("id", "4", "group_s", "group1", "test_ti", "10", "test_tl", "100"));
+ //
+ assertU(adoc("id", "5", "group_s", "group2", "test_ti", "5", "test_tl", "10", "term_s", "YYYY"));
+ assertU(commit());
+ assertU(adoc("id", "6", "group_s", "group2", "test_ti", "5", "test_tl","1000"));
+ assertU(adoc("id", "7", "group_s", "group2", "test_ti", "5", "test_tl","1000", "term_s", "XXXX"));
+ assertU(adoc("id", "8", "group_s", "group2", "test_ti", "10","test_tl", "100"));
+ assertU(commit());
+
+ // none of these grouping fields are in any doc
+ for (String group : new String[] {
+ "field=bogus_s", "field=bogus_s_dv",
+ "field=bogus_s hint=top_fc", // alternative docvalues codepath w/ hint
+ "field=bogus_s_dv hint=top_fc", // alternative docvalues codepath w/ hint
+ "field=bogus_ti", "field=bogus_tf" }) {
+
+ // for any of these selectors, behavior of these checks should be consistent
+ for (String selector : new String[] {
+ "", " sort='score desc' ",
+ " min=test_ti ", " max=test_ti ", " sort='test_ti asc' ", " sort='test_ti desc' ",
+ " min=test_tf ", " max=test_tf ", " sort='test_tf asc' ", " sort='test_tf desc' ",
+ " sort='group_s asc' ", " sort='group_s desc' ",
+ // fields that don't exist
+ " min=bogus_sort_ti ", " max=bogus_sort_ti ",
+ " sort='bogus_sort_ti asc' ", " sort='bogus_sort_ti desc' ",
+ " sort='bogus_sort_s asc' ", " sort='bogus_sort_s desc' ",
+ }) {
+
+
+ ModifiableSolrParams params = null;
+
+ // w/default nullPolicy, no groups found
+ params = new ModifiableSolrParams();
+ params.add("q", "*:*");
+ params.add("sort", "id desc");
+ params.add("fq", "{!collapse "+group+" "+selector+"}");
+ assertQ(req(params), "*[count(//doc)=0]");
+
+ // w/nullPolicy=expand, every doc found
+ params = new ModifiableSolrParams();
+ params.add("q", "*:*");
+ params.add("sort", "id desc");
+ params.add("fq", "{!collapse field="+group+" nullPolicy=expand "+selector+"}");
+ assertQ(req(params)
+ , "*[count(//doc)=8]"
+ ,"//result/doc[1]/float[@name='id'][.='8.0']"
+ ,"//result/doc[2]/float[@name='id'][.='7.0']"
+ ,"//result/doc[3]/float[@name='id'][.='6.0']"
+ ,"//result/doc[4]/float[@name='id'][.='5.0']"
+ ,"//result/doc[5]/float[@name='id'][.='4.0']"
+ ,"//result/doc[6]/float[@name='id'][.='3.0']"
+ ,"//result/doc[7]/float[@name='id'][.='2.0']"
+ ,"//result/doc[8]/float[@name='id'][.='1.0']"
+ );
+
+
+ }
+ }
+ }
+
public void testGroupHeadSelector() {
GroupHeadSelector s;
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRandomCollapseQParserPlugin.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRandomCollapseQParserPlugin.java?rev=1714701&r1=1714700&r2=1714701&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRandomCollapseQParserPlugin.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRandomCollapseQParserPlugin.java Mon Nov 16 22:42:25 2015
@@ -160,49 +160,53 @@ public class TestRandomCollapseQParserPl
"rows", "200",
"fq", ("{!collapse" + csize + nullPs +
" field="+collapseField+" sort='"+collapseSort+"'}"));
-
- final QueryResponse mainRsp = SOLR.query(SolrParams.wrapDefaults(collapseP, mainP));
- for (SolrDocument doc : mainRsp.getResults()) {
- final Object groupHeadId = doc.getFieldValue("id");
- final Object collapseVal = doc.getFieldValue(collapseField);
-
- if (null == collapseVal) {
- if (NULL_EXPAND.equals(nullPolicy)) {
- // nothing to check for this doc, it's in it's own group
- continue;
+ try {
+ final QueryResponse mainRsp = SOLR.query(SolrParams.wrapDefaults(collapseP, mainP));
+
+ for (SolrDocument doc : mainRsp.getResults()) {
+ final Object groupHeadId = doc.getFieldValue("id");
+ final Object collapseVal = doc.getFieldValue(collapseField);
+
+ if (null == collapseVal) {
+ if (NULL_EXPAND.equals(nullPolicy)) {
+ // nothing to check for this doc, it's in it's own group
+ continue;
+ }
+
+ assertFalse(groupHeadId + " has null collapseVal but nullPolicy==ignore; " +
+ "mainP: " + mainP + ", collapseP: " + collapseP,
+ NULL_IGNORE.equals(nullPolicy));
}
- assertFalse(groupHeadId + " has null collapseVal but nullPolicy==ignore; " +
- "mainP: " + mainP + ", collapseP: " + collapseP,
- NULL_IGNORE.equals(nullPolicy));
+ // work arround for SOLR-8082...
+ //
+ // what's important is that we already did the collapsing on the *real* collapseField
+ // to verify the groupHead returned is really the best our verification filter
+ // on docs with that value in a differnet ifeld containing the exact same values
+ final String checkField = collapseField.replace("float_dv", "float");
+
+ final String checkFQ = ((null == collapseVal)
+ ? ("-" + checkField + ":[* TO *]")
+ : ("{!field f="+checkField+"}" + collapseVal.toString()));
+
+ final SolrParams checkP = params("fq", checkFQ,
+ "rows", "1",
+ "sort", collapseSort);
+
+ final QueryResponse checkRsp = SOLR.query(SolrParams.wrapDefaults(checkP, mainP));
+
+ assertTrue("not even 1 match for sanity check query? expected: " + doc,
+ ! checkRsp.getResults().isEmpty());
+ final SolrDocument firstMatch = checkRsp.getResults().get(0);
+ final Object firstMatchId = firstMatch.getFieldValue("id");
+ assertEquals("first match for filtered group '"+ collapseVal +
+ "' not matching expected group head ... " +
+ "mainP: " + mainP + ", collapseP: " + collapseP + ", checkP: " + checkP,
+ groupHeadId, firstMatchId);
}
-
- // work arround for SOLR-8082...
- //
- // what's important is that we already did the collapsing on the *real* collapseField
- // to verify the groupHead returned is really the best our verification filter
- // on docs with that value in a differnet ifeld containing the exact same values
- final String checkField = collapseField.replace("float_dv", "float");
-
- final String checkFQ = ((null == collapseVal)
- ? ("-" + checkField + ":[* TO *]")
- : ("{!field f="+checkField+"}" + collapseVal.toString()));
-
- final SolrParams checkP = params("fq", checkFQ,
- "rows", "1",
- "sort", collapseSort);
-
- final QueryResponse checkRsp = SOLR.query(SolrParams.wrapDefaults(checkP, mainP));
-
- assertTrue("not even 1 match for sanity check query? expected: " + doc,
- ! checkRsp.getResults().isEmpty());
- final SolrDocument firstMatch = checkRsp.getResults().get(0);
- final Object firstMatchId = firstMatch.getFieldValue("id");
- assertEquals("first match for filtered group '"+ collapseVal +
- "' not matching expected group head ... " +
- "mainP: " + mainP + ", collapseP: " + collapseP + ", checkP: " + checkP,
- groupHeadId, firstMatchId);
+ } catch (Exception e) {
+ throw new RuntimeException("BUG using params: " + collapseP + " + " + mainP, e);
}
}
}