You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by yo...@apache.org on 2018/03/13 02:19:53 UTC
lucene-solr:branch_7_3: SOLR-12064: resize reused accs to fix bugs
with limit:-1 and missing:true
Repository: lucene-solr
Updated Branches:
refs/heads/branch_7_3 7046b9891 -> a348a8c46
SOLR-12064: resize reused accs to fix bugs with limit:-1 and missing:true
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/a348a8c4
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/a348a8c4
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/a348a8c4
Branch: refs/heads/branch_7_3
Commit: a348a8c46830010d00acbd6b8365a329179abe17
Parents: 7046b98
Author: yonik <yo...@apache.org>
Authored: Mon Mar 12 21:56:02 2018 -0400
Committer: yonik <yo...@apache.org>
Committed: Mon Mar 12 22:19:44 2018 -0400
----------------------------------------------------------------------
solr/CHANGES.txt | 4 +++
.../solr/search/facet/FacetFieldProcessor.java | 30 +++++++++++++++++---
.../solr/search/facet/TestJsonFacets.java | 14 +++++++--
3 files changed, 42 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/a348a8c4/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 088ce69..71f0997 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -255,6 +255,10 @@ Bug Fixes
* SOLR-12072: Invalid path string using ZkConfigManager.copyConfigDir(String fromConfig, String toConfig)
(Alessandro Hoss via Erick Erickson)
+* SOLR-12064: JSON Facet API: fix bug where a limit of -1 in conjunction with multiple facets or
+ missing=true caused an NPE or AIOOBE. (Karthik Ramachandran via yonik)
+
+
Optimizations
----------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/a348a8c4/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessor.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessor.java b/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessor.java
index 50f4676..9b47d66 100644
--- a/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessor.java
+++ b/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessor.java
@@ -83,9 +83,20 @@ abstract class FacetFieldProcessor extends FacetProcessor<FacetField> {
}
if (accs != null) {
- // reuse these accs, but reset them first
+ // reuse these accs, but reset them first and resize since size could be different
for (SlotAcc acc : accs) {
acc.reset();
+ acc.resize(new SlotAcc.Resizer() {
+ @Override
+ public int getNewSize() {
+ return slotCount;
+ }
+
+ @Override
+ public int getNewSlot(int oldSlot) {
+ return 0;
+ }
+ });
}
return;
} else {
@@ -339,11 +350,10 @@ abstract class FacetFieldProcessor extends FacetProcessor<FacetField> {
res.add("allBuckets", allBuckets);
}
+ SimpleOrderedMap<Object> missingBucket = new SimpleOrderedMap<>();
if (freq.missing) {
- // TODO: it would be more efficient to build up a missing DocSet if we need it here anyway.
- SimpleOrderedMap<Object> missingBucket = new SimpleOrderedMap<>();
- fillBucket(missingBucket, getFieldMissingQuery(fcontext.searcher, freq.field), null, false, null);
res.add("missing", missingBucket);
+ // moved missing fillBucket after we fill facet since it will reset all the accumulators.
}
// if we are deep paging, we don't have to order the highest "offset" counts.
@@ -371,6 +381,11 @@ abstract class FacetFieldProcessor extends FacetProcessor<FacetField> {
bucketList.add(bucket);
}
+ if (freq.missing) {
+ // TODO: it would be more efficient to build up a missing DocSet if we need it here anyway.
+ fillBucket(missingBucket, getFieldMissingQuery(fcontext.searcher, freq.field), null, false, null);
+ }
+
return res;
}
@@ -478,6 +493,13 @@ abstract class FacetFieldProcessor extends FacetProcessor<FacetField> {
}
@Override
+ public void setNextReader(LeafReaderContext ctx) throws IOException {
+ for (SlotAcc acc : subAccs) {
+ acc.setNextReader(ctx);
+ }
+ }
+
+ @Override
public void collect(int doc, int slot) throws IOException {
for (SlotAcc acc : subAccs) {
acc.collect(doc, slot);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/a348a8c4/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java b/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java
index 632c006..65d4e75 100644
--- a/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java
+++ b/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java
@@ -524,8 +524,8 @@ public class TestJsonFacets extends SolrTestCaseHS {
if (terms == null) terms="";
int limit=0;
switch (random().nextInt(4)) {
- case 0: limit=-1;
- case 1: limit=1000000;
+ case 0: limit=-1; break;
+ case 1: limit=1000000; break;
case 2: // fallthrough
case 3: // fallthrough
}
@@ -686,6 +686,16 @@ public class TestJsonFacets extends SolrTestCaseHS {
", f2:{ 'buckets':[{ val:'B', count:3, n1:-3.0}, { val:'A', count:2, n1:6.0 }]} }"
);
+ // test sorting by other stats and more than one facet
+ client.testJQ(params(p, "q", "*:*"
+ , "json.facet", "{f1:{terms:{${terms} field:'${cat_s}', sort:'n1 desc', facet:{n1:'sum(${num_d})', n2:'avg(${num_d})'} }}" +
+ " , f2:{terms:{${terms} field:'${cat_s}', sort:'n1 asc' , facet:{n1:'sum(${num_d})', n2:'avg(${num_d})'} }} }"
+ )
+ , "facets=={ 'count':6, " +
+ " f1:{ 'buckets':[{ val:'A', count:2, n1:6.0 , n2:3.0 }, { val:'B', count:3, n1:-3.0, n2:-1.0}]}" +
+ ", f2:{ 'buckets':[{ val:'B', count:3, n1:-3.0, n2:-1.0}, { val:'A', count:2, n1:6.0 , n2:3.0 }]} }"
+ );
+
// test sorting by other stats
client.testJQ(params(p, "q", "*:*"
, "json.facet", "{f1:{${terms} type:terms, field:'${cat_s}', sort:'x desc', facet:{x:'min(${num_d})'} }" +