diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index cb830ce9a6d..4b4820c3053 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -86,6 +86,9 @@ Bug Fixes * SOLR-11555: If the query terms reduce to nothing, filter(clause) produces an NPE whereas fq=clause does not (Erick Erickson) +* SOLR-11824: Fixed bucket ordering in distributed json.facet type:range when mincount>0 (hossman) + + Optimizations ---------------------- diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetRange.java b/solr/core/src/java/org/apache/solr/search/facet/FacetRange.java index b99b4b874fb..1176a77d879 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/FacetRange.java +++ b/solr/core/src/java/org/apache/solr/search/facet/FacetRange.java @@ -93,9 +93,13 @@ class FacetRangeProcessor extends FacetProcessor { super.process(); // Under the normal mincount=0, each shard will need to return 0 counts since we don't calculate buckets at the top level. - // But if mincount>0 then our sub mincount can be set to 1. - - effectiveMincount = fcontext.isShard() ? (freq.mincount > 0 ? 1 : 0) : freq.mincount; + // If mincount>0 then we could *potentially* set our sub mincount to 1... + // ...but that would require sorting the buckets (by their val) at the top level + // + // Tather then do that, which could be complicated by non trivial field types, we'll force the sub-shard effectiveMincount + // to be 0, ensuring that we can trivially merge all the buckets from every shard + // (we have to filter the merged buckets by the original mincount either way) + effectiveMincount = fcontext.isShard() ? 0 : freq.mincount; sf = fcontext.searcher.getSchema().getField(freq.field); response = getRangeCounts(); } diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetRangeMerger.java b/solr/core/src/java/org/apache/solr/search/facet/FacetRangeMerger.java index 5fae6c67490..6ddc05e06b9 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/FacetRangeMerger.java +++ b/solr/core/src/java/org/apache/solr/search/facet/FacetRangeMerger.java @@ -45,7 +45,7 @@ public class FacetRangeMerger extends FacetRequestSortedMerger { @Override public void sortBuckets() { - // TODO: mincount>0 will mess up order? + // regardless of mincount, every shard returns a consistent set of buckets which are already in the correct order sortedBuckets = new ArrayList<>( buckets.values() ); } 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 bf709cfe54a..753c7dcc03e 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 @@ -1028,6 +1028,48 @@ public class TestJsonFacets extends SolrTestCaseHS { ",between:{count:3,x:0.0, ny:{count:2}}" + " } }" ); + + // sparse range facet (with sub facets and stats), with "other:all" + client.testJQ(params(p, "q", "*:*", "json.facet", + "{f:{range:{field:${num_d}, start:-5, end:10, gap:1, other:all, "+ + " facet:{ x:'sum(${num_i})', ny:{query:'${where_s}:NY'}} }}}" + ) + , "facets=={count:6, f:{buckets:[ {val:-5.0,count:1, x:-5.0,ny:{count:1}}, "+ + " {val:-4.0,count:0 /* ,x:0.0,ny:{count:0} */} ,"+ + " {val:-3.0,count:0 /* ,x:0.0,ny:{count:0} */} ,"+ + " {val:-2.0,count:0 /* ,x:0.0,ny:{count:0} */} ,"+ + " {val:-1.0,count:0 /* ,x:0.0,ny:{count:0} */} ,"+ + " {val: 0.0,count:0 /* ,x:0.0,ny:{count:0} */} ,"+ + " {val: 1.0,count:0 /* ,x:0.0,ny:{count:0} */} ,"+ + " {val: 2.0,count:1, x:3.0,ny:{count:0}} , "+ + " {val: 3.0,count:0 /* ,x:0.0,ny:{count:0} */} ,"+ + " {val: 4.0,count:1, x:2.0,ny:{count:1}} , "+ + " {val: 5.0,count:0 /* ,x:0.0,ny:{count:0} */} ,"+ + " {val: 6.0,count:0 /* ,x:0.0,ny:{count:0} */} ,"+ + " {val: 7.0,count:0 /* ,x:0.0,ny:{count:0} */} ,"+ + " {val: 8.0,count:0 /* ,x:0.0,ny:{count:0} */} ,"+ + " {val: 9.0,count:0 /* ,x:0.0,ny:{count:0} */}"+ + " ]" + + " ,before: {count:1,x:-5.0,ny:{count:0}}" + + " ,after: {count:1,x:7.0, ny:{count:0}}" + + " ,between:{count:3,x:0.0, ny:{count:2}}" + + " } }" + ); + + // sparse range facet (with sub facets and stats), with "other:all" & mincount==1 + client.testJQ(params(p, "q", "*:*", "json.facet", + "{f:{range:{field:${num_d}, start:-5, end:10, gap:1, other:all, mincount:1, "+ + " facet:{ x:'sum(${num_i})', ny:{query:'${where_s}:NY'}} }}}" + ) + , "facets=={count:6, f:{buckets:[ {val:-5.0,count:1, x:-5.0,ny:{count:1}}, "+ + " {val: 2.0,count:1, x:3.0,ny:{count:0}} , "+ + " {val: 4.0,count:1, x:2.0,ny:{count:1}} "+ + " ]" + + " ,before: {count:1,x:-5.0,ny:{count:0}}" + + " ,after: {count:1,x:7.0, ny:{count:0}}" + + " ,between:{count:3,x:0.0, ny:{count:2}}" + + " } }" + ); // range facet with sub facets and stats, with "other:all", on subset client.testJQ(params(p, "q", "id:(3 4 6)"