lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hoss...@apache.org
Subject lucene-solr:master: SOLR-12516: Fix some bugs in 'type:range' Facet refinement when sub-facets are combined with non default values for the 'other' and 'include' options.
Date Fri, 06 Jul 2018 16:34:15 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/master a09f3facf -> 7d8ef9e39


SOLR-12516: Fix some bugs in 'type:range' Facet refinement when sub-facets are combined with non default values for the 'other' and 'include' options.

1) the optional other buckets (before/after/between) are not considered during refinement

2) when using the include option: if edge is specified, then the refinement of all range buckets mistakenly includes the lower bound of the range, regardless of whether lower was specified.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/7d8ef9e3
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/7d8ef9e3
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/7d8ef9e3

Branch: refs/heads/master
Commit: 7d8ef9e39d3321a5366fcfe1a358ec015fb7b8b1
Parents: a09f3fa
Author: Chris Hostetter <hossman@apache.org>
Authored: Fri Jul 6 09:34:05 2018 -0700
Committer: Chris Hostetter <hossman@apache.org>
Committed: Fri Jul 6 09:34:05 2018 -0700

----------------------------------------------------------------------
 solr/CHANGES.txt                                |   2 +
 .../apache/solr/search/facet/FacetRange.java    | 378 +++++----
 .../solr/search/facet/FacetRangeMerger.java     |  67 +-
 .../search/CurrencyRangeFacetCloudTest.java     |   9 +-
 .../solr/search/facet/RangeFacetCloudTest.java  | 786 +++++++++++++++++++
 .../search/facet/TestJsonFacetRefinement.java   |  98 ++-
 .../solr/search/facet/TestJsonFacets.java       |  73 +-
 7 files changed, 1225 insertions(+), 188 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7d8ef9e3/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index b113aaf..91456ee 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -140,6 +140,8 @@ Bug Fixes
 
 * SOLR-2834: Fix SolrJ Field and Document analyzes for types that include CharacterFilter (Alexandre Rafalovitch)
 
+* SOLR-12516: Fix some bugs in 'type:range' Facet refinement when sub-facets are combined with non
+  default values for the 'other' and 'include' options. (hossman)
 
 Optimizations
 ----------------------

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7d8ef9e3/solr/core/src/java/org/apache/solr/search/facet/FacetRange.java
----------------------------------------------------------------------
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 817ada6..6366a9b 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
@@ -27,7 +27,8 @@ import java.util.Map;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.util.NumericUtils;
 import org.apache.solr.common.SolrException;
-import org.apache.solr.common.params.FacetParams;
+import org.apache.solr.common.params.FacetParams.FacetRangeInclude;
+import org.apache.solr.common.params.FacetParams.FacetRangeOther;
 import org.apache.solr.common.util.SimpleOrderedMap;
 import org.apache.solr.schema.CurrencyFieldType;
 import org.apache.solr.schema.CurrencyValue;
@@ -44,13 +45,15 @@ import org.apache.solr.util.DateMathParser;
 import static org.apache.solr.search.facet.FacetContext.SKIP_FACET;
 
 public class FacetRange extends FacetRequestSorted {
+  static final String ACTUAL_END_JSON_KEY = "_actual_end";
+  
   String field;
   Object start;
   Object end;
   Object gap;
   boolean hardend = false;
-  EnumSet<FacetParams.FacetRangeInclude> include;
-  EnumSet<FacetParams.FacetRangeOther> others;
+  EnumSet<FacetRangeInclude> include;
+  EnumSet<FacetRangeOther> others;
 
   {
     // defaults
@@ -82,30 +85,68 @@ public class FacetRange extends FacetRequestSorted {
 
 
 class FacetRangeProcessor extends FacetProcessor<FacetRange> {
-  SchemaField sf;
-  Calc calc;
+  // TODO: the code paths for initial faceting, vs refinement, are very different...
+  // TODO: ...it might make sense to have seperate classes w/a common base?
+  // TODO: let FacetRange.createFacetProcessor decide which one to instantiate?
+  
+  final SchemaField sf;
+  final Calc calc;
+  final EnumSet<FacetRangeInclude> include;
+  final long effectiveMincount;
+  final Comparable start;
+  final Comparable end;
+  final String gap;
+  
+  /** Build by {@link #createRangeList} if and only if needed for basic faceting */
   List<Range> rangeList;
+  /** Build by {@link #createRangeList} if and only if needed for basic faceting */
   List<Range> otherList;
-  long effectiveMincount;
+
+  /**
+   * Serves two purposes depending on the type of request.
+   * <ul>
+   * <li>If this is a phase#1 shard request, then {@link #createRangeList} will set this value (non null)
+   *     if and only if it is needed for refinement (ie: <code>hardend:false</code> &amp; <code>other</code>
+   *     that requres an end value low/high value calculation).  And it wil be included in the response</li>
+   * <li>If this is a phase#2 refinement request, this variable will be used 
+   *     {@link #getOrComputeActualEndForRefinement} to track the value sent with the refinement request 
+   *     -- or to cache a recomputed value if the request omitted it -- for use in refining the 
+   *     <code>other</code> buckets that need them</li>
+   * </ul>
+   */
+  Comparable actual_end = null; // null until/unless we need it
 
   FacetRangeProcessor(FacetContext fcontext, FacetRange freq) {
     super(fcontext, freq);
-  }
-
-  @Override
-  public void process() throws IOException {
-    super.process();
+    include = freq.include;
+    sf = fcontext.searcher.getSchema().getField(freq.field);
+    calc = getCalcForField(sf);
+    start = calc.getValue(freq.start.toString());
+    end = calc.getValue(freq.end.toString());
+    gap = freq.gap.toString();
 
+    
     // Under the normal mincount=0, each shard will need to return 0 counts since we don't calculate buckets at the top level.
     // 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
+    // Rather 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();
+  }
+
+  @Override
+  public void process() throws IOException {
+    super.process();
+
+    if (fcontext.facetInfo != null) { // refinement?
+      response = refineFacets();
+    } else {
+      // phase#1: build list of all buckets and return full facets...
+      createRangeList();
+      response = getRangeCountsIndexed();
+    }
   }
 
   private static class Range {
@@ -126,7 +167,7 @@ class FacetRangeProcessor extends FacetProcessor<FacetRange> {
 
   /**
    * Returns a {@link Calc} instance to use for <em>term</em> faceting over a numeric field.
-   * This metod is unused for <code>range</code> faceting, and exists solely as a helper method for other classes
+   * This method is unused for <code>range</code> faceting, and exists solely as a helper method for other classes
    * 
    * @param sf A field to facet on, must be of a type such that {@link FieldType#getNumberType} is non null
    * @return a <code>Calc</code> instance with {@link Calc#bitsToValue} and {@link Calc#bitsToSortableBits} methods suitable for the specified field.
@@ -190,68 +231,53 @@ class FacetRangeProcessor extends FacetProcessor<FacetRange> {
     return calc;
   }
 
-  private SimpleOrderedMap<Object> getRangeCounts() throws IOException {
+  /**
+   * Helper method used in processor constructor
+   * @return a <code>Calc</code> instance with {@link Calc#bitsToValue} and {@link Calc#bitsToSortableBits} methods suitable for the specified field.
+   */
+  private static Calc getCalcForField(SchemaField sf) {
     final FieldType ft = sf.getType();
-
     if (ft instanceof TrieField || ft.isPointField()) {
       switch (ft.getNumberType()) {
         case FLOAT:
-          calc = new FloatCalc(sf);
-          break;
+          return new FloatCalc(sf);
         case DOUBLE:
-          calc = new DoubleCalc(sf);
-          break;
+          return new DoubleCalc(sf);
         case INTEGER:
-          calc = new IntCalc(sf);
-          break;
+          return new IntCalc(sf);
         case LONG:
-          calc = new LongCalc(sf);
-          break;
+          return new LongCalc(sf);
         case DATE:
-          calc = new DateCalc(sf, null);
-          break;
+          return new DateCalc(sf, null);
         default:
           throw new SolrException
               (SolrException.ErrorCode.BAD_REQUEST,
-                  "Unable to range facet on tried field of unexpected type:" + freq.field);
+                  "Unable to range facet on numeric field of unexpected type:" + sf.getName());
       }
     } else if (ft instanceof CurrencyFieldType) {
-      calc = new CurrencyCalc(sf);
-    } else {
-      throw new SolrException
-          (SolrException.ErrorCode.BAD_REQUEST,
-              "Unable to range facet on field:" + sf);
+      return new CurrencyCalc(sf);
     }
 
-    if (fcontext.facetInfo != null) {
-      return refineFacets();
-    }
-
-    createRangeList();
-    return getRangeCountsIndexed();
+    // if we made it this far, we have no idea what it is...
+    throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+                            "Unable to range facet on field:" + sf.getName());
   }
 
-
   private void createRangeList() throws IOException {
 
     rangeList = new ArrayList<>();
     otherList = new ArrayList<>(3);
 
-    Comparable start = calc.getValue(freq.start.toString());
-    Comparable end = calc.getValue(freq.end.toString());
-    EnumSet<FacetParams.FacetRangeInclude> include = freq.include;
-
-    String gap = freq.gap.toString();
-
     Comparable low = start;
-
+    Comparable loop_end = this.end;
+    
     while (low.compareTo(end) < 0) {
       Comparable high = calc.addGap(low, gap);
       if (end.compareTo(high) < 0) {
         if (freq.hardend) {
-          high = end;
+          high = loop_end;
         } else {
-          end = high;
+          loop_end = high;
         }
       }
       if (high.compareTo(low) < 0) {
@@ -265,15 +291,11 @@ class FacetRangeProcessor extends FacetProcessor<FacetRange> {
                 "range facet infinite loop: gap is either zero, or too small relative start/end and caused underflow: " + low + " + " + gap + " = " + high );
       }
 
-      boolean incLower =
-          (include.contains(FacetParams.FacetRangeInclude.LOWER) ||
-              (include.contains(FacetParams.FacetRangeInclude.EDGE) &&
-                  0 == low.compareTo(start)));
-      boolean incUpper =
-          (include.contains(FacetParams.FacetRangeInclude.UPPER) ||
-              (include.contains(FacetParams.FacetRangeInclude.EDGE) &&
-                  0 == high.compareTo(end)));
-
+      boolean incLower =(include.contains(FacetRangeInclude.LOWER) ||
+                         (include.contains(FacetRangeInclude.EDGE) && 0 == low.compareTo(start)));
+      boolean incUpper = (include.contains(FacetRangeInclude.UPPER) ||
+                          (include.contains(FacetRangeInclude.EDGE) && 0 == high.compareTo(end)));
+      
       Range range = new Range(calc.buildRangeLabel(low), low, high, incLower, incUpper);
       rangeList.add( range );
 
@@ -282,37 +304,28 @@ class FacetRangeProcessor extends FacetProcessor<FacetRange> {
 
     // no matter what other values are listed, we don't do
     // anything if "none" is specified.
-    if (! freq.others.contains(FacetParams.FacetRangeOther.NONE) ) {
-
-      boolean all = freq.others.contains(FacetParams.FacetRangeOther.ALL);
+    if (! freq.others.contains(FacetRangeOther.NONE) ) {
+      final boolean all = freq.others.contains(FacetRangeOther.ALL);
 
-      if (all || freq.others.contains(FacetParams.FacetRangeOther.BEFORE)) {
-        // include upper bound if "outer" or if first gap doesn't already include it
-        boolean incUpper = (include.contains(FacetParams.FacetRangeInclude.OUTER) ||
-            (!(include.contains(FacetParams.FacetRangeInclude.LOWER) ||
-                include.contains(FacetParams.FacetRangeInclude.EDGE))));
-        otherList.add( new Range(FacetParams.FacetRangeOther.BEFORE.toString(), null, start, false, incUpper) );
+      if (all || freq.others.contains(FacetRangeOther.BEFORE)) {
+        otherList.add( buildBeforeRange() );
       }
-      if (all || freq.others.contains(FacetParams.FacetRangeOther.AFTER)) {
-        // include lower bound if "outer" or if last gap doesn't already include it
-        boolean incLower = (include.contains(FacetParams.FacetRangeInclude.OUTER) ||
-            (!(include.contains(FacetParams.FacetRangeInclude.UPPER) ||
-                include.contains(FacetParams.FacetRangeInclude.EDGE))));
-        otherList.add( new Range(FacetParams.FacetRangeOther.AFTER.toString(), end, null, incLower, false));
+      if (all || freq.others.contains(FacetRangeOther.AFTER)) {
+        actual_end = loop_end;
+        otherList.add( buildAfterRange() );
       }
-      if (all || freq.others.contains(FacetParams.FacetRangeOther.BETWEEN)) {
-        boolean incLower = (include.contains(FacetParams.FacetRangeInclude.LOWER) ||
-            include.contains(FacetParams.FacetRangeInclude.EDGE));
-        boolean incUpper = (include.contains(FacetParams.FacetRangeInclude.UPPER) ||
-            include.contains(FacetParams.FacetRangeInclude.EDGE));
-
-        otherList.add( new Range(FacetParams.FacetRangeOther.BETWEEN.toString(), start, end, incLower, incUpper) );
+      if (all || freq.others.contains(FacetRangeOther.BETWEEN)) {
+        actual_end = loop_end;
+        otherList.add( buildBetweenRange() );
       }
     }
-
+    // if we're not a shard request, or this is a hardend:true situation, then actual_end isn't needed
+    if (freq.hardend || (! fcontext.isShard())) {
+      actual_end = null;
+    }
   }
-
-
+  
+  
   private  SimpleOrderedMap getRangeCountsIndexed() throws IOException {
 
     int slotCount = rangeList.size() + otherList.size();
@@ -353,6 +366,10 @@ class FacetRangeProcessor extends FacetProcessor<FacetRange> {
       addStats(bucket, rangeList.size() + idx);
       doSubs(bucket, rangeList.size() + idx);
     }
+      
+    if (null != actual_end) {
+      res.add(FacetRange.ACTUAL_END_JSON_KEY, calc.formatValue(actual_end));
+    }
 
     return res;
   }
@@ -382,23 +399,6 @@ class FacetRangeProcessor extends FacetProcessor<FacetRange> {
     }
   }
 
-  private  SimpleOrderedMap<Object> rangeStats(Range range, boolean special ) throws IOException {
-    SimpleOrderedMap<Object> bucket = new SimpleOrderedMap<>();
-
-    // typically the start value of the range, but null for before/after/between
-    if (!special) {
-      bucket.add("val", range.label);
-    }
-
-    Query rangeQ = sf.getType().getRangeQuery(null, sf, range.low == null ? null : calc.formatValue(range.low), range.high==null ? null : calc.formatValue(range.high), range.includeLower, range.includeUpper);
-    fillBucket(bucket, rangeQ, null, false, null);
-
-    return bucket;
-  }
-
-
-
-
   // Essentially copied from SimpleFacets...
   // would be nice to unify this stuff w/ analytics component...
   /**
@@ -712,34 +712,31 @@ class FacetRangeProcessor extends FacetProcessor<FacetRange> {
 
   }
 
-  // this refineFacets method is patterned after FacetFieldProcessor.refineFacets and should
-  // probably be merged when range facet becomes more like field facet in it's ability to sort and limit
   protected SimpleOrderedMap<Object> refineFacets() throws IOException {
+    // this refineFacets method is patterned after FacetFieldProcessor.refineFacets such that
+    // the same "_s" skip bucket syntax is used and FacetRangeMerger can subclass FacetRequestSortedMerger
+    // for dealing with them & the refinement requests.
+    // 
+    // But range faceting does *NOT* use the "leaves" and "partial" syntax
+    // 
+    // If/When range facet becomes more like field facet in it's ability to sort and limit the "range buckets"
+    // FacetRangeProcessor and FacetFieldProcessor should prbably be refactored to share more code.
+    
     boolean skipThisFacet = (fcontext.flags & SKIP_FACET) != 0;
 
-    List leaves = FacetFieldProcessor.asList(fcontext.facetInfo.get("_l"));        // We have not seen this bucket: do full faceting for this bucket, including all sub-facets
     List<List> skip = FacetFieldProcessor.asList(fcontext.facetInfo.get("_s"));    // We have seen this bucket, so skip stats on it, and skip sub-facets except for the specified sub-facets that should calculate specified buckets.
-    List<List> partial = FacetFieldProcessor.asList(fcontext.facetInfo.get("_p")); // We have not seen this bucket, do full faceting for this bucket, and most sub-facets... but some sub-facets are partial and should only visit specified buckets.
-
-    // currently, only _s should be present for range facets.  In the future, range facets will
-    // be more like field facets and will have the same refinement cases.  When that happens, we should try to unify the refinement code more
-    assert leaves.size() == 0;
-    assert partial.size() == 0;
 
-    // For leaf refinements, we do full faceting for each leaf bucket.  Any sub-facets of these buckets will be fully evaluated.  Because of this, we should never
-    // encounter leaf refinements that have sub-facets that return partial results.
+    // sanity check our merger's super class didn't send us something we can't handle ...
+    assert 0 == FacetFieldProcessor.asList(fcontext.facetInfo.get("_l")).size();
+    assert 0 == FacetFieldProcessor.asList(fcontext.facetInfo.get("_p")).size();
 
     SimpleOrderedMap<Object> res = new SimpleOrderedMap<>();
-    List<SimpleOrderedMap> bucketList = new ArrayList<>( leaves.size() + skip.size() + partial.size() );
+    List<SimpleOrderedMap> bucketList = new ArrayList<>( skip.size() );
     res.add("buckets", bucketList);
 
     // TODO: an alternate implementations can fill all accs at once
     createAccs(-1, 1);
 
-    for (Object bucketVal : leaves) {
-      bucketList.add( refineBucket(bucketVal, false, null) );
-    }
-
     for (List bucketAndFacetInfo : skip) {
       assert bucketAndFacetInfo.size() == 2;
       Object bucketVal = bucketAndFacetInfo.get(0);
@@ -748,51 +745,78 @@ class FacetRangeProcessor extends FacetProcessor<FacetRange> {
       bucketList.add( refineBucket(bucketVal, true, facetInfo ) );
     }
 
-    // The only difference between skip and missing is the value of "skip" passed to refineBucket
-    for (List bucketAndFacetInfo : partial) {
-      assert bucketAndFacetInfo.size() == 2;
-      Object bucketVal = bucketAndFacetInfo.get(0);
-      Map<String,Object> facetInfo = (Map<String, Object>) bucketAndFacetInfo.get(1);
-
-      bucketList.add( refineBucket(bucketVal, false, facetInfo ) );
-    }
-
-    /*** special buckets
-    if (freq.missing) {
-      Map<String,Object> bucketFacetInfo = (Map<String,Object>)fcontext.facetInfo.get("missing");
+    { // refine the special "other" buckets
+      
+      // NOTE: we're re-useing this variable for each special we look for...
+      Map<String,Object> specialFacetInfo;
 
-      if (bucketFacetInfo != null || !skipThisFacet) {
-        SimpleOrderedMap<Object> missingBucket = new SimpleOrderedMap<>();
-        fillBucket(missingBucket, getFieldMissingQuery(fcontext.searcher, freq.field), null, skipThisFacet, bucketFacetInfo);
-        res.add("missing", missingBucket);
+      specialFacetInfo = (Map<String, Object>) fcontext.facetInfo.get(FacetRangeOther.BEFORE.toString());
+      if (null != specialFacetInfo) {
+        res.add(FacetRangeOther.BEFORE.toString(),
+                refineRange(buildBeforeRange(), skipThisFacet, specialFacetInfo));
+      }
+      
+      specialFacetInfo = (Map<String, Object>) fcontext.facetInfo.get(FacetRangeOther.AFTER.toString());
+      if (null != specialFacetInfo) {
+        res.add(FacetRangeOther.AFTER.toString(),
+                refineRange(buildAfterRange(), skipThisFacet, specialFacetInfo));
+      }
+      
+      specialFacetInfo = (Map<String, Object>) fcontext.facetInfo.get(FacetRangeOther.BETWEEN.toString());
+      if (null != specialFacetInfo) {
+        res.add(FacetRangeOther.BETWEEN.toString(),
+                refineRange(buildBetweenRange(), skipThisFacet, specialFacetInfo));
       }
     }
-     **********/
-
-
-    // If there are just a couple of leaves, and if the domain is large, then
-    // going by term is likely the most efficient?
-    // If the domain is small, or if the number of leaves is large, then doing
-    // the normal collection method may be best.
-
+      
     return res;
   }
 
+  /** 
+   * Returns the "Actual End" value sent from the merge as part of the refinement request (if any) 
+   * or re-computes it as needed using the Calc and caches the result for re-use
+   */
+  private Comparable getOrComputeActualEndForRefinement() {
+    if (null != actual_end) {
+      return actual_end;
+    }
+    
+    if (freq.hardend) {
+      actual_end = this.end;
+    } else if (fcontext.facetInfo.containsKey(FacetRange.ACTUAL_END_JSON_KEY)) {
+      actual_end = calc.getValue(fcontext.facetInfo.get(FacetRange.ACTUAL_END_JSON_KEY).toString());
+    } else {
+      // a quick and dirty loop over the ranges (we don't need) to compute the actual_end...
+      Comparable low = start;
+      while (low.compareTo(end) < 0) {
+        Comparable high = calc.addGap(low, gap);
+        if (end.compareTo(high) < 0) {
+          actual_end = high;
+          break;
+        }
+        if (high.compareTo(low) <= 0) {
+          throw new SolrException
+            (SolrException.ErrorCode.BAD_REQUEST,
+             "Garbage input for facet refinement w/o " + FacetRange.ACTUAL_END_JSON_KEY);
+        }
+        low = high;
+      }
+    }
+    
+    assert null != actual_end;
+    return actual_end;
+  }
+  
   private SimpleOrderedMap<Object> refineBucket(Object bucketVal, boolean skip, Map<String,Object> facetInfo) throws IOException {
-    // TODO: refactor this repeated code from above
-    Comparable start = calc.getValue(bucketVal.toString());
-    Comparable end = calc.getValue(freq.end.toString());
-    EnumSet<FacetParams.FacetRangeInclude> include = freq.include;
-
-    String gap = freq.gap.toString();
 
     Comparable low = calc.getValue(bucketVal.toString());
     Comparable high = calc.addGap(low, gap);
+    Comparable max_end = end;
     if (end.compareTo(high) < 0) {
       if (freq.hardend) {
-        high = end;
+        high = max_end;
       } else {
-        end = high;
+        max_end = high;
       }
     }
     if (high.compareTo(low) < 0) {
@@ -806,26 +830,58 @@ class FacetRangeProcessor extends FacetProcessor<FacetRange> {
               "range facet infinite loop: gap is either zero, or too small relative start/end and caused underflow: " + low + " + " + gap + " = " + high );
     }
 
-    boolean incLower =
-        (include.contains(FacetParams.FacetRangeInclude.LOWER) ||
-            (include.contains(FacetParams.FacetRangeInclude.EDGE) &&
-                0 == low.compareTo(start)));
-    boolean incUpper =
-        (include.contains(FacetParams.FacetRangeInclude.UPPER) ||
-            (include.contains(FacetParams.FacetRangeInclude.EDGE) &&
-                0 == high.compareTo(end)));
+    boolean incLower = (include.contains(FacetRangeInclude.LOWER) ||
+                        (include.contains(FacetRangeInclude.EDGE) && 0 == low.compareTo(start)));
+    boolean incUpper = (include.contains(FacetRangeInclude.UPPER) ||
+                        (include.contains(FacetRangeInclude.EDGE) && 0 == high.compareTo(max_end)));
 
     Range range = new Range(calc.buildRangeLabel(low), low, high, incLower, incUpper);
 
-
     // now refine this range
 
-    SimpleOrderedMap<Object> bucket = new SimpleOrderedMap<>();
+    final SimpleOrderedMap<Object> bucket = refineRange(range, skip, facetInfo);
     bucket.add("val", range.label);
-    
-    Query domainQ = sf.getType().getRangeQuery(null, sf, range.low == null ? null : calc.formatValue(range.low), range.high==null ? null : calc.formatValue(range.high), range.includeLower, range.includeUpper);
-    fillBucket(bucket, domainQ, null, skip, facetInfo);
 
     return bucket;
   }
+
+  /** Helper method for refining a Range
+   * @see #fillBucket
+   */
+  private SimpleOrderedMap<Object> refineRange(Range range, boolean skip, Map<String,Object> facetInfo) throws IOException {
+    final SimpleOrderedMap<Object> bucket = new SimpleOrderedMap<>();
+    final Query domainQ = sf.getType().getRangeQuery(null, sf, range.low == null ? null : calc.formatValue(range.low), range.high==null ? null : calc.formatValue(range.high), range.includeLower, range.includeUpper);
+    fillBucket(bucket, domainQ, null, skip, facetInfo);
+    return bucket;
+  }
+  
+  /** Helper method for building a "before" Range */
+  private Range buildBeforeRange() {
+    // include upper bound if "outer" or if first gap doesn't already include it
+    final boolean incUpper = (include.contains(FacetRangeInclude.OUTER) ||
+                              (!(include.contains(FacetRangeInclude.LOWER) ||
+                                 include.contains(FacetRangeInclude.EDGE))));
+    return new Range(FacetRangeOther.BEFORE.toString(), null, start, false, incUpper);
+  }
+
+  /** Helper method for building a "after" Range */
+  private Range buildAfterRange() {
+    final Comparable the_end = getOrComputeActualEndForRefinement();
+    assert null != the_end;
+    final boolean incLower = (include.contains(FacetRangeInclude.OUTER) ||
+                              (!(include.contains(FacetRangeInclude.UPPER) ||
+                                 include.contains(FacetRangeInclude.EDGE))));
+    return new Range(FacetRangeOther.AFTER.toString(), the_end, null, incLower, false);
+  }
+
+  /** Helper method for building a "between" Range */
+  private Range buildBetweenRange() {
+    final Comparable the_end = getOrComputeActualEndForRefinement();
+    assert null != the_end;
+    final boolean incLower = (include.contains(FacetRangeInclude.LOWER) ||
+                              include.contains(FacetRangeInclude.EDGE));
+    final boolean incUpper = (include.contains(FacetRangeInclude.UPPER) ||
+                              include.contains(FacetRangeInclude.EDGE));
+    return new Range(FacetRangeOther.BETWEEN.toString(), start, the_end, incLower, incUpper);
+  }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7d8ef9e3/solr/core/src/java/org/apache/solr/search/facet/FacetRangeMerger.java
----------------------------------------------------------------------
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 a3bfdc8..452652f 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
@@ -19,7 +19,10 @@
 package org.apache.solr.search.facet;
 
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.apache.solr.common.params.FacetParams;
 import org.apache.solr.common.util.SimpleOrderedMap;
@@ -28,17 +31,13 @@ public class FacetRangeMerger extends FacetRequestSortedMerger<FacetRange> {
   FacetBucket beforeBucket;
   FacetBucket afterBucket;
   FacetBucket betweenBucket;
+  Object actual_end = null;
 
   public FacetRangeMerger(FacetRange freq) {
     super(freq);
   }
 
   @Override
-  FacetMerger createFacetMerger(String key, Object val) {
-    return super.createFacetMerger(key, val);
-  }
-
-  @Override
   public void merge(Object facetResult, Context mcontext) {
     super.merge(facetResult, mcontext);
     merge((SimpleOrderedMap) facetResult , mcontext);
@@ -54,12 +53,47 @@ public class FacetRangeMerger extends FacetRequestSortedMerger<FacetRange> {
   public void finish(Context mcontext) {
     // nothing to do
   }
-
+  
+  @Override
+  Map<String, Object> getRefinementSpecial(Context mcontext, Map<String, Object> refinement, Collection<String> tagsWithPartial) {
+    if (!tagsWithPartial.isEmpty()) {
+      // Since 'other' buckets will always be included, we only need to worry about subfacets being partial.
+
+      refinement = getRefinementSpecial(mcontext, refinement, tagsWithPartial, beforeBucket, FacetParams.FacetRangeOther.BEFORE.toString());
+      refinement = getRefinementSpecial(mcontext, refinement, tagsWithPartial, afterBucket, FacetParams.FacetRangeOther.AFTER.toString());
+      refinement = getRefinementSpecial(mcontext, refinement, tagsWithPartial, betweenBucket, FacetParams.FacetRangeOther.BETWEEN.toString());
+
+      // if we need an actual end to compute either of these buckets,
+      // and it's been returned to us by at least one shard
+      // send it back as part of the refinement request
+      if ( (!freq.hardend) &&
+           actual_end != null &&
+           refinement != null &&
+           (refinement.containsKey(FacetParams.FacetRangeOther.AFTER.toString()) ||
+            refinement.containsKey(FacetParams.FacetRangeOther.BETWEEN.toString())) ) {
+        refinement.put("_actual_end", actual_end);
+      }
+    }
+    return refinement;
+  }
+  
+  private Map<String, Object> getRefinementSpecial(Context mcontext, Map<String, Object> refinement, Collection<String> tagsWithPartial, FacetBucket bucket, String label) {
+    if (null == bucket) {
+      return refinement;
+    }
+    Map<String, Object> bucketRefinement = bucket.getRefinement(mcontext, tagsWithPartial);
+    if (bucketRefinement != null) {
+      refinement = refinement == null ? new HashMap<>(2) : refinement;
+      refinement.put(label, bucketRefinement);
+    }
+    return refinement;
+  }
+  
   public void merge(SimpleOrderedMap facetResult, Context mcontext) {
     boolean all = freq.others.contains(FacetParams.FacetRangeOther.ALL);
 
     if (all || freq.others.contains(FacetParams.FacetRangeOther.BEFORE)) {
-      Object o = facetResult.get("before");
+      Object o = facetResult.get(FacetParams.FacetRangeOther.BEFORE.toString());
       if (o != null) {
         if (beforeBucket == null) {
           beforeBucket = newBucket(null, mcontext);
@@ -69,7 +103,7 @@ public class FacetRangeMerger extends FacetRequestSortedMerger<FacetRange> {
     }
 
     if (all || freq.others.contains(FacetParams.FacetRangeOther.AFTER)) {
-      Object o = facetResult.get("after");
+      Object o = facetResult.get(FacetParams.FacetRangeOther.AFTER.toString());
       if (o != null) {
         if (afterBucket == null) {
           afterBucket = newBucket(null, mcontext);
@@ -79,7 +113,7 @@ public class FacetRangeMerger extends FacetRequestSortedMerger<FacetRange> {
     }
 
     if (all || freq.others.contains(FacetParams.FacetRangeOther.BETWEEN)) {
-      Object o = facetResult.get("between");
+      Object o = facetResult.get(FacetParams.FacetRangeOther.BETWEEN.toString());
       if (o != null) {
         if (betweenBucket == null) {
           betweenBucket = newBucket(null, mcontext);
@@ -88,6 +122,15 @@ public class FacetRangeMerger extends FacetRequestSortedMerger<FacetRange> {
       }
     }
 
+    Object shard_actual_end = facetResult.get(FacetRange.ACTUAL_END_JSON_KEY);
+    if (null != shard_actual_end) {
+      if (null == actual_end) {
+        actual_end = shard_actual_end;
+      } else {
+        assert actual_end.equals(shard_actual_end) : actual_end + " != " + shard_actual_end;
+      }
+    }
+
     List<SimpleOrderedMap> bucketList = (List<SimpleOrderedMap>) facetResult.get("buckets");
     mergeBucketList(bucketList , mcontext);
   }
@@ -110,13 +153,13 @@ public class FacetRangeMerger extends FacetRequestSortedMerger<FacetRange> {
     result.add("buckets", resultBuckets);
 
     if (beforeBucket != null) {
-      result.add("before", beforeBucket.getMergedBucket());
+      result.add(FacetParams.FacetRangeOther.BEFORE.toString(), beforeBucket.getMergedBucket());
     }
     if (afterBucket != null) {
-      result.add("after", afterBucket.getMergedBucket());
+      result.add(FacetParams.FacetRangeOther.AFTER.toString(), afterBucket.getMergedBucket());
     }
     if (betweenBucket != null) {
-      result.add("between", betweenBucket.getMergedBucket());
+      result.add(FacetParams.FacetRangeOther.BETWEEN.toString(), betweenBucket.getMergedBucket());
     }
     return result;
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7d8ef9e3/solr/core/src/test/org/apache/solr/search/CurrencyRangeFacetCloudTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/CurrencyRangeFacetCloudTest.java b/solr/core/src/test/org/apache/solr/search/CurrencyRangeFacetCloudTest.java
index c4b9281..652f3f0 100644
--- a/solr/core/src/test/org/apache/solr/search/CurrencyRangeFacetCloudTest.java
+++ b/solr/core/src/test/org/apache/solr/search/CurrencyRangeFacetCloudTest.java
@@ -95,7 +95,6 @@ public class CurrencyRangeFacetCloudTest extends SolrCloudTestCase {
   }
 
   public void testSimpleRangeFacetsOfSymetricRates() throws Exception {
-
     for (boolean use_mincount : Arrays.asList(true, false)) {
     
       // exchange rates relative to USD...
@@ -201,7 +200,6 @@ public class CurrencyRangeFacetCloudTest extends SolrCloudTestCase {
         } catch (AssertionError|RuntimeException ae) {
           throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
         }
-        
       }
     }
   }
@@ -342,10 +340,15 @@ public class CurrencyRangeFacetCloudTest extends SolrCloudTestCase {
     // the *facet* results should be the same regardless of wether we filter via fq, or using a domain filter on the top facet
     for (boolean use_domain : Arrays.asList(true, false)) {
       final String domain = use_domain ? "domain: { filter:'" + filter + "'}," : "";
+
+      // both of these options should produce same results since hardened:false is default
+      final String end = random().nextBoolean() ? "end:'20,EUR'" : "end:'15,EUR'";
+        
+      
       final SolrQuery solrQuery = new SolrQuery("q", (use_domain ? "*:*" : filter),
                                                 "rows", "0", "json.facet",
                                                 "{ bar:{ type:range, field:"+FIELD+", " + domain + 
-                                                "        start:'0,EUR', gap:'10,EUR', end:'20,EUR', other:all " +
+                                                "        start:'0,EUR', gap:'10,EUR', "+end+", other:all " +
                                                 "        facet: { foo:{ type:terms, field:x_s, " +
                                                 "                       refine:true, limit:2, overrequest:0" +
                                                 " } } } }");

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7d8ef9e3/solr/core/src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java b/solr/core/src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java
new file mode 100644
index 0000000..05c25cf
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java
@@ -0,0 +1,786 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search.facet;
+
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.stream.Collectors;
+
+import org.apache.lucene.util.TestUtil;
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.common.params.FacetParams.FacetRangeOther;
+import org.apache.solr.common.util.NamedList;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.junit.BeforeClass;
+
+/**
+ * Builds a random index of a few simple fields, maintaining an in-memory model of the expected
+ * doc counts so that we can verify the results of range facets w/ nested field facets that need refinement.
+ *
+ * The focus here is on stressing the casees where the document values fall directonly on the 
+ * range boundaries, and how the various "include" options affects refinement.
+ */
+public class RangeFacetCloudTest extends SolrCloudTestCase {
+
+  private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+  
+  private static final String COLLECTION = MethodHandles.lookup().lookupClass().getName();
+  private static final String CONF = COLLECTION + "_configSet";
+
+  private static final String INT_FIELD = "range_i";
+  private static final String STR_FIELD = "facet_s";
+  private static final int NUM_RANGE_VALUES = 6;
+  private static final int TERM_VALUES_RANDOMIZER = 100;
+
+  // TODO: add 'count asc' once SOLR-12343 is fixed
+  private static final List<String> SORTS = Arrays.asList("count desc", "index asc", "index desc");
+  
+  private static final List<EnumSet<FacetRangeOther>> OTHERS = buildListOfFacetRangeOtherOptions();
+  private static final List<FacetRangeOther> BEFORE_AFTER_BETWEEN
+    = Arrays.asList(FacetRangeOther.BEFORE, FacetRangeOther.AFTER, FacetRangeOther.BETWEEN);
+    
+  /**
+   * the array indexes represent values in our numeric field, while the array values
+   * track the number of docs that will have that value.
+   */
+  private static final int[] RANGE_MODEL = new int[NUM_RANGE_VALUES];
+  /**
+   * the array indexes represent values in our numeric field, while the array values
+   * track the mapping from string field terms to facet counts for docs that have that numeric value
+   */
+  private static final Map<String,Integer>[] TERM_MODEL = new Map[NUM_RANGE_VALUES];
+  
+  @BeforeClass
+  public static void setupCluster() throws Exception {
+    final int numShards = TestUtil.nextInt(random(),1,5);
+    final int numReplicas = 1;
+    final int maxShardsPerNode = 1;
+    final int nodeCount = numShards * numReplicas;
+
+    configureCluster(nodeCount)
+      .addConfig(CONF, Paths.get(TEST_HOME(), "collection1", "conf"))
+      .configure();
+
+    assertEquals(0, (CollectionAdminRequest.createCollection(COLLECTION, CONF, numShards, numReplicas)
+                     .setMaxShardsPerNode(maxShardsPerNode)
+                     .setProperties(Collections.singletonMap(CoreAdminParams.CONFIG, "solrconfig-minimal.xml"))
+                     .process(cluster.getSolrClient())).getStatus());
+    
+    cluster.getSolrClient().setDefaultCollection(COLLECTION);
+
+    final int numDocs = atLeast(1000);
+    final int maxTermId = atLeast(TERM_VALUES_RANDOMIZER);
+    
+    // seed the TERM_MODEL Maps so we don't have null check later
+    for (int i = 0; i < NUM_RANGE_VALUES; i++) {
+      TERM_MODEL[i] = new LinkedHashMap<>();
+    }
+
+    // build our index & our models
+    for (int id = 0; id < numDocs; id++) {
+      final int rangeVal = random().nextInt(NUM_RANGE_VALUES);
+      final String termVal = "x" + random().nextInt(maxTermId);
+      final SolrInputDocument doc = sdoc("id", ""+id,
+                                         INT_FIELD, ""+rangeVal,
+                                         STR_FIELD, termVal);
+      RANGE_MODEL[rangeVal]++;
+      TERM_MODEL[rangeVal].merge(termVal, 1, Integer::sum);
+
+      assertEquals(0, (new UpdateRequest().add(doc)).process(cluster.getSolrClient()).getStatus());
+    }
+    assertEquals(0, cluster.getSolrClient().commit().getStatus());
+    
+  }
+
+  public void testInclude_Lower() throws Exception {
+    for (boolean doSubFacet : Arrays.asList(false, true)) {
+      final Integer subFacetLimit = pickSubFacetLimit(doSubFacet);
+      final CharSequence subFacet = makeSubFacet(subFacetLimit);
+      for (EnumSet<FacetRangeOther> other : OTHERS) {
+        final String otherStr = formatFacetRangeOther(other);
+        for (String include : Arrays.asList(", include:lower", "")) { // same behavior
+          final SolrQuery solrQuery = new SolrQuery
+            ("q", "*:*", "rows", "0", "json.facet",
+             // exclude a single low value from our ranges
+             "{ foo:{ type:range, field:"+INT_FIELD+" start:1, end:5, gap:1"+otherStr+include+subFacet+" } }");
+        
+          final QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+          try {
+            final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+            final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+            
+            assertEquals("num buckets", 4, buckets.size());
+            for (int i = 0; i < 4; i++) {
+              int expectedVal = i+1;
+              assertBucket("bucket#" + i, expectedVal, modelVals(expectedVal), subFacetLimit, buckets.get(i));
+            }
+            
+            assertBeforeAfterBetween(other, modelVals(0), modelVals(5), modelVals(1,4), subFacetLimit, foo);
+            
+          } catch (AssertionError|RuntimeException ae) {
+            throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+          }
+        }
+      }
+    }
+  }
+  
+  public void testInclude_Lower_Gap2() throws Exception {
+    for (boolean doSubFacet : Arrays.asList(false, true)) {
+      final Integer subFacetLimit = pickSubFacetLimit(doSubFacet);
+      final CharSequence subFacet = makeSubFacet(subFacetLimit);
+      for (EnumSet<FacetRangeOther> other : OTHERS) {
+        final String otherStr = formatFacetRangeOther(other);
+        for (String include : Arrays.asList(", include:lower", "")) { // same behavior
+          final SolrQuery solrQuery = new SolrQuery
+            ("q", "*:*", "rows", "0", "json.facet",
+             "{ foo:{ type:range, field:"+INT_FIELD+" start:0, end:5, gap:2"+otherStr+include+subFacet+" } }");
+        
+          final QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+          try {
+            final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+            final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+            
+            assertEquals("num buckets", 3, buckets.size());
+            assertBucket("bucket#0", 0, modelVals(0,1), subFacetLimit, buckets.get(0));
+            assertBucket("bucket#1", 2, modelVals(2,3), subFacetLimit, buckets.get(1));
+            assertBucket("bucket#2", 4, modelVals(4,5), subFacetLimit, buckets.get(2));
+            
+            assertBeforeAfterBetween(other, emptyVals(), emptyVals(), modelVals(0,5), subFacetLimit, foo);
+            
+          } catch (AssertionError|RuntimeException ae) {
+            throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+          }
+        }
+      }
+    }
+  }
+  public void testInclude_Lower_Gap2_hardend() throws Exception {
+    for (boolean doSubFacet : Arrays.asList(false, true)) {
+      final Integer subFacetLimit = pickSubFacetLimit(doSubFacet);
+      final CharSequence subFacet = makeSubFacet(subFacetLimit);
+      for (EnumSet<FacetRangeOther> other : OTHERS) {
+        final String otherStr = formatFacetRangeOther(other);
+        for (String include : Arrays.asList(", include:lower", "")) { // same behavior
+          final SolrQuery solrQuery = new SolrQuery
+            ("q", "*:*", "rows", "0", "json.facet",
+             "{ foo:{ type:range, field:"+INT_FIELD+" start:0, end:5, gap:2, hardend:true"
+             +        otherStr+include+subFacet+" } }");
+        
+          final QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+          try {
+            final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+            final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+            
+            assertEquals("num buckets", 3, buckets.size());
+            assertBucket("bucket#0", 0, modelVals(0,1), subFacetLimit, buckets.get(0));
+            assertBucket("bucket#1", 2, modelVals(2,3), subFacetLimit, buckets.get(1));
+            assertBucket("bucket#2", 4, modelVals(4), subFacetLimit, buckets.get(2));
+            
+            assertBeforeAfterBetween(other, emptyVals(), modelVals(5), modelVals(0,4), subFacetLimit, foo);
+            
+          } catch (AssertionError|RuntimeException ae) {
+            throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+          }
+        }
+      }
+    }
+  }
+  
+  public void testInclude_Upper() throws Exception {
+    for (boolean doSubFacet : Arrays.asList(false, true)) {
+      final Integer subFacetLimit = pickSubFacetLimit(doSubFacet);
+      final CharSequence subFacet = makeSubFacet(subFacetLimit);
+      for (EnumSet<FacetRangeOther> other : OTHERS) {
+        final String otherStr = formatFacetRangeOther(other);
+        final SolrQuery solrQuery = new SolrQuery
+          ("q", "*:*", "rows", "0", "json.facet",
+           // exclude a single high value from our ranges
+           "{ foo:{ type:range, field:"+INT_FIELD+" start:0, end:4, gap:1, include:upper"+otherStr+subFacet+" } }");
+    
+        final QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+        try {
+          final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+          final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+          
+          assertEquals("num buckets", 4, buckets.size());
+          for (int i = 0; i < 4; i++) {
+            assertBucket("bucket#" + i, i, modelVals(i+1), subFacetLimit, buckets.get(i));
+          }
+          assertBeforeAfterBetween(other, modelVals(0), modelVals(5), modelVals(1,4), subFacetLimit, foo);
+          
+        } catch (AssertionError|RuntimeException ae) {
+          throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+        }
+      }
+    }
+  }
+  public void testInclude_Upper_Gap2() throws Exception {
+    for (boolean doSubFacet : Arrays.asList(false, true)) {
+      final Integer subFacetLimit = pickSubFacetLimit(doSubFacet);
+      final CharSequence subFacet = makeSubFacet(subFacetLimit);
+      for (EnumSet<FacetRangeOther> other : OTHERS) {
+        final String otherStr = formatFacetRangeOther(other);
+        final SolrQuery solrQuery = new SolrQuery
+          ("q", "*:*", "rows", "0", "json.facet",
+           // exclude a single high value from our ranges
+           "{ foo:{ type:range, field:"+INT_FIELD+" start:0, end:4, gap:2, include:upper"+otherStr+subFacet+" } }");
+    
+        final QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+        try {
+          final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+          final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+          
+          assertEquals("num buckets", 2, buckets.size());
+          assertBucket("bucket#0", 0, modelVals(1,2), subFacetLimit, buckets.get(0));
+          assertBucket("bucket#1", 2, modelVals(3,4), subFacetLimit, buckets.get(1));
+          
+          assertBeforeAfterBetween(other, modelVals(0), modelVals(5), modelVals(1,4), subFacetLimit, foo);
+          
+        } catch (AssertionError|RuntimeException ae) {
+          throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+        }
+      }
+    }
+  }
+  
+  public void testInclude_Edge() throws Exception {
+    for (boolean doSubFacet : Arrays.asList(false, true)) {
+      final Integer subFacetLimit = pickSubFacetLimit(doSubFacet);
+      final CharSequence subFacet = makeSubFacet(subFacetLimit);
+      for (EnumSet<FacetRangeOther> other : OTHERS) {
+        final String otherStr = formatFacetRangeOther(other);
+        final SolrQuery solrQuery = new SolrQuery
+          ("q", "*:*", "rows", "0", "json.facet",
+           // exclude a single low/high value from our ranges
+           "{ foo:{ type:range, field:"+INT_FIELD+" start:1, end:4, gap:1, include:edge"+otherStr+subFacet+" } }");
+        
+        final QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+        try {
+          final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+          final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+          
+          assertEquals("num buckets", 3, buckets.size());
+          
+          assertBucket("bucket#0", 1, modelVals(1), subFacetLimit, buckets.get(0));
+          
+          // middle bucket doesn't include lower or upper so it's empty
+          assertBucket("bucket#1", 2, emptyVals(), subFacetLimit, buckets.get(1));
+          
+          assertBucket("bucket#2", 3, modelVals(4), subFacetLimit, buckets.get(2));
+          
+          assertBeforeAfterBetween(other, modelVals(0), modelVals(5), modelVals(1,4), subFacetLimit, foo);
+          
+        } catch (AssertionError|RuntimeException ae) {
+          throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+        }
+      }
+    }
+  }
+
+  public void testInclude_EdgeLower() throws Exception {
+    for (boolean doSubFacet : Arrays.asList(false, true)) {
+      final Integer subFacetLimit = pickSubFacetLimit(doSubFacet);
+      final CharSequence subFacet = makeSubFacet(subFacetLimit);
+      for (EnumSet<FacetRangeOther> other : OTHERS) {
+        final String otherStr = formatFacetRangeOther(other);
+        for (String include : Arrays.asList(", include:'edge,lower'", ", include:[edge,lower]")) { // same
+          final SolrQuery solrQuery = new SolrQuery
+            ("q", "*:*", "rows", "0", "json.facet",
+             // exclude a single low/high value from our ranges
+             "{ foo:{ type:range, field:"+INT_FIELD+" start:1, end:4, gap:1"+otherStr+include+subFacet+" } }");
+          
+          final QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+          try {
+            final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+            final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+            
+            assertEquals("num buckets", 3, buckets.size());
+            
+            assertBucket("bucket#0", 1, modelVals(1), subFacetLimit, buckets.get(0));
+            assertBucket("bucket#1", 2, modelVals(2), subFacetLimit, buckets.get(1));
+            assertBucket("bucket#2", 3, modelVals(3,4), subFacetLimit, buckets.get(2));
+            
+            assertBeforeAfterBetween(other, modelVals(0), modelVals(5), modelVals(1,4), subFacetLimit, foo);
+            
+          } catch (AssertionError|RuntimeException ae) {
+            throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+          }
+        }
+      }
+    }
+  }
+  
+  public void testInclude_EdgeUpper() throws Exception {
+    for (boolean doSubFacet : Arrays.asList(false, true)) {
+      final Integer subFacetLimit = pickSubFacetLimit(doSubFacet);
+      final CharSequence subFacet = makeSubFacet(subFacetLimit);
+      for (EnumSet<FacetRangeOther> other : OTHERS) {
+        final String otherStr = formatFacetRangeOther(other);
+        for (String include : Arrays.asList(", include:'edge,upper'", ", include:[edge,upper]")) { // same
+          final SolrQuery solrQuery = new SolrQuery
+            ("q", "*:*", "rows", "0", "json.facet",
+             // exclude a single low/high value from our ranges
+             "{ foo:{ type:range, field:"+INT_FIELD+" start:1, end:4, gap:1"+otherStr+include+subFacet+" } }");
+          
+          final QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+          try {
+            final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+            final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+            
+            assertEquals("num buckets", 3, buckets.size());
+            
+            assertBucket("bucket#0", 1, modelVals(1,2), subFacetLimit, buckets.get(0));
+            assertBucket("bucket#1", 2, modelVals(3), subFacetLimit, buckets.get(1));
+            assertBucket("bucket#2", 3, modelVals(4), subFacetLimit, buckets.get(2));
+            
+            assertBeforeAfterBetween(other, modelVals(0), modelVals(5), modelVals(1,4), subFacetLimit, foo);
+            
+          } catch (AssertionError|RuntimeException ae) {
+            throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+          }
+        }
+      }
+    }
+  }
+  
+  public void testInclude_EdgeLowerUpper() throws Exception {
+    for (boolean doSubFacet : Arrays.asList(false, true)) {
+      final Integer subFacetLimit = pickSubFacetLimit(doSubFacet);
+      final CharSequence subFacet = makeSubFacet(subFacetLimit);
+      for (EnumSet<FacetRangeOther> other : OTHERS) {
+        final String otherStr = formatFacetRangeOther(other);
+        for (String include : Arrays.asList(", include:'edge,lower,upper'", ", include:[edge,lower,upper]")) { // same
+          final SolrQuery solrQuery = new SolrQuery
+            ("q", "*:*", "rows", "0", "json.facet",
+             // exclude a single low/high value from our ranges
+             "{ foo:{ type:range, field:"+INT_FIELD+" start:1, end:4, gap:1"+otherStr+include+subFacet+" } }");
+          
+          final QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+          try {
+            final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+            final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+            
+            assertEquals("num buckets", 3, buckets.size());
+            
+            assertBucket("bucket#0", 1, modelVals(1,2), subFacetLimit, buckets.get(0));
+            assertBucket("bucket#1", 2, modelVals(2,3), subFacetLimit, buckets.get(1));
+            assertBucket("bucket#2", 3, modelVals(3,4), subFacetLimit, buckets.get(2));
+            
+            assertBeforeAfterBetween(other, modelVals(0), modelVals(5), modelVals(1,4), subFacetLimit, foo);
+            
+          } catch (AssertionError|RuntimeException ae) {
+            throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+          }
+        }
+      }
+    }
+  }
+  
+  public void testInclude_All() throws Exception {
+    for (boolean doSubFacet : Arrays.asList(false, true)) {
+      final Integer subFacetLimit = pickSubFacetLimit(doSubFacet);
+      final CharSequence subFacet = makeSubFacet(subFacetLimit);
+      for (EnumSet<FacetRangeOther> other : OTHERS) {
+        final String otherStr = formatFacetRangeOther(other);
+        for (String include : Arrays.asList(", include:'edge,lower,upper,outer'",
+                                            ", include:[edge,lower,upper,outer]",
+                                            ", include:all")) { // same
+          final SolrQuery solrQuery = new SolrQuery
+            ("q", "*:*", "rows", "0", "json.facet",
+             // exclude a single low/high value from our ranges
+             "{ foo:{ type:range, field:"+INT_FIELD+" start:1, end:4, gap:1"+otherStr+include+subFacet+" } }");
+          
+          final QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+          try {
+            final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+            final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+            
+            assertEquals("num buckets", 3, buckets.size());
+            
+            assertBucket("bucket#0", 1, modelVals(1,2), subFacetLimit, buckets.get(0));
+            assertBucket("bucket#1", 2, modelVals(2,3), subFacetLimit, buckets.get(1));
+            assertBucket("bucket#2", 3, modelVals(3,4), subFacetLimit, buckets.get(2));
+            
+            assertBeforeAfterBetween(other, modelVals(0,1), modelVals(4,5), modelVals(1,4), subFacetLimit, foo);
+            
+          } catch (AssertionError|RuntimeException ae) {
+            throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+          }
+        }
+      }
+    }
+  }
+
+  /**
+   * This test will also sanity check that mincount is working properly 
+   */
+  public void testInclude_All_Gap2() throws Exception {
+    for (boolean doSubFacet : Arrays.asList(false, true)) {
+      final Integer subFacetLimit = pickSubFacetLimit(doSubFacet);
+      final CharSequence subFacet = makeSubFacet(subFacetLimit);
+      for (EnumSet<FacetRangeOther> other : OTHERS) {
+        final String otherStr = formatFacetRangeOther(other);
+        for (String include : Arrays.asList(", include:'edge,lower,upper,outer'",
+                                            ", include:[edge,lower,upper,outer]",
+                                            ", include:all")) { // same
+
+          // we also want to sanity check that mincount doesn't bork anything,
+          // so we're going to do the query twice:
+          // 1) no mincount, keep track of which bucket has the highest count & what it was
+          // 2) use that value as the mincount, assert that the other bucket isn't returned
+          long mincount_to_use = -1;
+          Object expected_mincount_bucket_val = null; // HACK: use null to mean neither in case of tie
+
+          // initial query, no mincount...
+          SolrQuery solrQuery = new SolrQuery
+            ("q", "*:*", "rows", "0", "json.facet",
+             // exclude a single low/high value from our ranges
+             "{ foo:{ type:range, field:"+INT_FIELD+", start:1, end:4, gap:2"+otherStr+include+subFacet+" } }");
+          
+          QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+          try {
+            final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+            final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+            
+            assertEquals("num buckets", 2, buckets.size());
+            
+            assertBucket("bucket#0", 1, modelVals(1,3), subFacetLimit, buckets.get(0));
+            assertBucket("bucket#1", 3, modelVals(3,5), subFacetLimit, buckets.get(1));
+            
+            assertBeforeAfterBetween(other, modelVals(0,1), modelVals(5), modelVals(1,5), subFacetLimit, foo);
+
+            // if we've made it this far, then our buckets match the model
+            // now use our buckets to pick a mincount to use based on the MIN(+1) count seen
+            long count0 = ((Number)buckets.get(0).get("count")).longValue();
+            long count1 = ((Number)buckets.get(1).get("count")).longValue();
+            
+            mincount_to_use = 1 + Math.min(count0, count1);
+            if (count0 > count1) {
+              expected_mincount_bucket_val = buckets.get(0).get("val");
+            } else if (count1 > count0) {
+              expected_mincount_bucket_val = buckets.get(1).get("val");
+            }
+
+          } catch (AssertionError|RuntimeException ae) {
+            throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+          }
+
+          // second query, using mincount...
+          solrQuery = new SolrQuery
+            ("q", "*:*", "rows", "0", "json.facet",
+             // exclude a single low/high value from our ranges, 
+             "{ foo:{ type:range, field:"+INT_FIELD+", mincount:" + mincount_to_use +
+             ", start:1, end:4, gap:2"+otherStr+include+subFacet+" } }");
+          
+          rsp = cluster.getSolrClient().query(solrQuery);
+          try {
+            final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+            final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+
+            if (null == expected_mincount_bucket_val) {
+              assertEquals("num buckets", 0, buckets.size());
+            } else {
+              assertEquals("num buckets", 1, buckets.size());
+              final Object actualBucket = buckets.get(0);
+              if (expected_mincount_bucket_val.equals(1)) {
+                assertBucket("bucket#0(0)", 1, modelVals(1,3), subFacetLimit, actualBucket);
+              } else {
+                assertBucket("bucket#0(1)", 3, modelVals(3,5), subFacetLimit, actualBucket);
+              }
+            }
+            
+            // regardless of mincount, the before/after/between special buckets should always be returned
+            assertBeforeAfterBetween(other, modelVals(0,1), modelVals(5), modelVals(1,5), subFacetLimit, foo);
+
+          } catch (AssertionError|RuntimeException ae) {
+            throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+          }
+
+
+
+          
+        }
+      }
+    }
+  }
+  
+  public void testInclude_All_Gap2_hardend() throws Exception {
+    for (boolean doSubFacet : Arrays.asList(false, true)) {
+      final Integer subFacetLimit = pickSubFacetLimit(doSubFacet);
+      final CharSequence subFacet = makeSubFacet(subFacetLimit);
+      for (EnumSet<FacetRangeOther> other : OTHERS) {
+        final String otherStr = formatFacetRangeOther(other);
+        for (String include : Arrays.asList(", include:'edge,lower,upper,outer'",
+                                            ", include:[edge,lower,upper,outer]",
+                                            ", include:all")) { // same
+          final SolrQuery solrQuery = new SolrQuery
+            ("q", "*:*", "rows", "0", "json.facet",
+             // exclude a single low/high value from our ranges
+             "{ foo:{ type:range, field:"+INT_FIELD+" start:1, end:4, gap:2, hardend:true"
+             +         otherStr+include+subFacet+" } }");
+          
+          final QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+          try {
+            final NamedList<Object> foo = ((NamedList<NamedList<Object>>)rsp.getResponse().get("facets")).get("foo");
+            final List<NamedList<Object>> buckets = (List<NamedList<Object>>) foo.get("buckets");
+            
+            assertEquals("num buckets", 2, buckets.size());
+            
+            assertBucket("bucket#0", 1, modelVals(1,3), subFacetLimit, buckets.get(0));
+            assertBucket("bucket#1", 3, modelVals(3,4), subFacetLimit, buckets.get(1));
+            
+            assertBeforeAfterBetween(other, modelVals(0,1), modelVals(4,5), modelVals(1,4), subFacetLimit, foo);
+            
+          } catch (AssertionError|RuntimeException ae) {
+            throw new AssertionError(solrQuery.toString() + " -> " + rsp.toString() + " ===> " + ae.getMessage(), ae);
+          }
+        }
+      }
+    }
+  }
+
+  /**
+   * Helper method for validating a single 'bucket' from a Range facet.
+   *
+   * @param label to use in assertions
+   * @param expectedVal <code>"val"</code> to assert for this bucket, use <code>null</code> for special "buckets" like before, after, between.
+   * @param expectedRangeValues a range of the expected values in the numeric field whose cumulative counts should match this buckets <code>"count"</code>
+   * @param subFacetLimitUsed if null, then assert this bucket has no  <code>"bar"</code> subfacet, otherwise assert expected term counts for each actual term, and sanity check the number terms returnd against the model and/or this limit.
+   * @param actualBucket the actual bucket returned from a query for all assertions to be conducted against.
+   */
+  private static void assertBucket(final String label,
+                                   final Integer expectedVal,
+                                   final ModelRange expectedRangeValues,
+                                   final Integer subFacetLimitUsed,
+                                   final Object actualBucket) {
+    try {
+      
+      assertNotNull("null bucket", actualBucket);
+      assertNotNull("expectedRangeValues", expectedRangeValues);
+      assertTrue("bucket is not a NamedList", actualBucket instanceof NamedList);
+      final NamedList<Object> bucket = (NamedList<Object>) actualBucket;
+
+      if (null != expectedVal) {
+        assertEquals("val", expectedVal, bucket.get("val"));
+      }
+      
+      // figure out the model from our range of values...
+      long expectedCount = 0;
+      List<Map<String,Integer>> toMerge = new ArrayList<>(NUM_RANGE_VALUES);
+      for (int i = expectedRangeValues.lower; i <= expectedRangeValues.upper; i++) {
+        expectedCount += RANGE_MODEL[i];
+        toMerge.add(TERM_MODEL[i]);
+      }
+      
+      assertEqualsHACK("count", expectedCount, bucket.get("count"));
+      
+      // merge the maps of our range values by summing the (int) values on key collisions
+      final Map<String,Long> expectedTermCounts = toMerge.stream()
+        .flatMap(m -> m.entrySet().stream())
+        .collect(Collectors.toMap(Entry::getKey, (e -> e.getValue().longValue()), Long::sum));
+
+      if (null == subFacetLimitUsed || 0 == expectedCount) {
+        assertNull("unexpected subfacets", bucket.get("bar"));
+      } else {
+        NamedList<Object> bar = ((NamedList<Object>)bucket.get("bar"));
+        assertNotNull("can't find subfacet 'bar'", bar);
+
+        final int numBucketsExpected = subFacetLimitUsed < 0
+          ? expectedTermCounts.size() : Math.min(subFacetLimitUsed, expectedTermCounts.size());
+        final List<NamedList<Object>> subBuckets = (List<NamedList<Object>>) bar.get("buckets");
+        // we should either have filled out the expected limit, or 
+        assertEquals("num subfacet buckets", numBucketsExpected, subBuckets.size());
+
+        // assert sub-facet term counts for the subBuckets that do exist
+        for (NamedList<Object> subBucket : subBuckets) {
+          final Object term = subBucket.get("val");
+          assertNotNull("subfacet bucket with null term: " + subBucket, term);
+          final Long expectedTermCount = expectedTermCounts.get(term.toString());
+          assertNotNull("unexpected subfacet bucket: " + subBucket, expectedTermCount);
+          assertEqualsHACK("subfacet count for term: " + term, expectedTermCount, subBucket.get("count"));
+        }
+      }
+        
+    } catch (AssertionError|RuntimeException ae) {
+      throw new AssertionError(label + ": " + ae.getMessage(), ae);
+    }
+  }
+  
+  /**
+   * A convinience method for calling {@link #assertBucket} on the before/after/between buckets 
+   * of a facet result, based on the {@link FacetRangeOther} specified for this facet.
+   * 
+   * @see #assertBucket
+   * @see #buildListOfFacetRangeOtherOptions 
+   */
+  private static void assertBeforeAfterBetween(final EnumSet<FacetRangeOther> other,
+                                               final ModelRange before,
+                                               final ModelRange after,
+                                               final ModelRange between,
+                                               final Integer subFacetLimitUsed,
+                                               final NamedList<Object> facet) {
+    //final String[] names = new String[] { "before", "after", "between" };
+    assertEquals(3, BEFORE_AFTER_BETWEEN.size());
+    final ModelRange[] expected = new ModelRange[] { before, after, between };
+    for (int i = 0; i < 3; i++) {
+      FacetRangeOther key = BEFORE_AFTER_BETWEEN.get(i);
+      String name = key.toString();
+      if (other.contains(key) || other.contains(FacetRangeOther.ALL)) {
+        assertBucket(name, null, expected[i], subFacetLimitUsed, facet.get(name));
+      } else {
+        assertNull("unexpected other=" + name, facet.get(name));
+      }
+    }
+  }
+
+  /** 
+   * A little helper struct to make the method sig of {@link #assertBucket} more readable.
+   * If lower (or upper) is negative, then both must be negative and upper must be less then 
+   * lower -- this indicate that the bucket should be empty.
+   * @see #modelVals
+   * @see #emptyVals
+   */
+  private static final class ModelRange {
+    public final int lower;
+    public final int upper;
+    /** Don't use, use the convinience methods */
+    public ModelRange(int lower, int upper) {
+      if (lower < 0 || upper < 0) {
+        assert(lower < 0 && upper < lower);
+      } else {
+        assert(lower <= upper);
+      }
+      this.lower = lower;
+      this.upper = upper;
+    }
+  }
+  private static final ModelRange emptyVals() {
+    return new ModelRange(-1, -100);
+  }
+  private static final ModelRange modelVals(int value) {
+    return modelVals(value, value);
+  }
+  private static final ModelRange modelVals(int lower, int upper) {
+    assertTrue(upper + " < " + lower, lower <= upper);
+    assertTrue("negative lower", 0 <= lower);
+    assertTrue("negative upper", 0 <= upper);
+    return new ModelRange(lower, upper);
+  }
+
+  /** randomized helper */
+  private static final Integer pickSubFacetLimit(final boolean doSubFacet) {
+    if (! doSubFacet) { return null; }
+    int result = TestUtil.nextInt(random(), -10, atLeast(TERM_VALUES_RANDOMIZER));
+    return (result <= 0) ? -1 : result;
+  }
+  /** randomized helper */
+  private static final CharSequence makeSubFacet(final Integer subFacetLimit) {
+    if (null == subFacetLimit) {
+      return "";
+    }
+    final StringBuilder result = new StringBuilder(", facet:{ bar:{ type:terms, refine:true, field:"+STR_FIELD);
+    // constrain overrequesting to stress refiement, but still test those codepaths
+    final String overrequest = random().nextBoolean() ? "0" : "1";
+      
+    result.append(", overrequest:").append(overrequest).append(", limit:").append(subFacetLimit);
+    
+    // order should have no affect on our testing
+    if (random().nextBoolean()) {
+      result.append(", sort:'").append(SORTS.get(random().nextInt(SORTS.size()))).append("'");
+    }
+    result.append("} }");
+    return result;
+  }
+
+  /** 
+   * Helper for seeding the re-used static struct, and asserting no one changes the Enum w/o updating this test
+   *
+   * @see #assertBeforeAfterBetween 
+   * @see #formatFacetRangeOther
+   * @see #OTHERS
+   */
+  private static final List<EnumSet<FacetRangeOther>> buildListOfFacetRangeOtherOptions() {
+    assertEquals("If someone adds to FacetRangeOther this method (and bulk of test) needs updated",
+                 5, EnumSet.allOf(FacetRangeOther.class).size());
+    
+    // we're not overly concerned about testing *EVERY* permutation,
+    // we just want to make sure we test multiple code paths (some, all, "ALL", none)
+    //
+    // NOTE: Don't mix "ALL" or "NONE" with other options so we don't have to make assertBeforeAfterBetween
+    // overly complicated
+    ArrayList<EnumSet<FacetRangeOther>> results = new ArrayList(5);
+    results.add(EnumSet.of(FacetRangeOther.ALL));
+    results.add(EnumSet.of(FacetRangeOther.BEFORE, FacetRangeOther.AFTER, FacetRangeOther.BETWEEN));
+    results.add(EnumSet.of(FacetRangeOther.BEFORE, FacetRangeOther.AFTER));
+    results.add(EnumSet.of(FacetRangeOther.BETWEEN));
+    results.add(EnumSet.of(FacetRangeOther.NONE));
+    return results;
+  }
+  
+  /** 
+   * @see #assertBeforeAfterBetween 
+   * @see #buildListOfFacetRangeOtherOptions
+   */
+  private static final String formatFacetRangeOther(EnumSet<FacetRangeOther> other) {
+    if (other.contains(FacetRangeOther.NONE) && random().nextBoolean()) {
+      return ""; // sometimes don't output a param at all when we're dealing with the default NONE
+    }
+    String val = other.toString();
+    if (random().nextBoolean()) {
+      // two valid syntaxes to randomize between:
+      // - a JSON list of items (conviniently the default toString of EnumSet),
+      // - a single quoted string containing the comma seperated list
+      val = val.replaceAll("\\[|\\]","'");
+
+      // HACK: work around SOLR-12539...
+      //
+      // when sending a single string containing a comma seperated list of values, JSON Facets 'other'
+      // parsing can't handle any leading (or trailing?) whitespace
+      val = val.replaceAll("\\s","");
+    }
+    return ", other:" + val;
+  }
+  
+  /**
+   * HACK to work around SOLR-11775.
+   * Asserts that the 'actual' argument is a (non-null) Number, then compares it's 'longValue' to the 'expected' argument
+   */
+  private static void assertEqualsHACK(String msg, long expected, Object actual) {
+    assertNotNull(msg, actual);
+    assertTrue(msg + " ... NOT A NUMBER: " + actual.getClass(), Number.class.isInstance(actual));
+    assertEquals(msg, expected, ((Number)actual).longValue());
+  }
+  
+}

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7d8ef9e3/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacetRefinement.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacetRefinement.java b/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacetRefinement.java
index 779e58b..2e68294 100644
--- a/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacetRefinement.java
+++ b/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacetRefinement.java
@@ -18,6 +18,7 @@
 package org.apache.solr.search.facet;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.List;
 
 import org.apache.solr.JSONTestUtil;
@@ -227,6 +228,64 @@ public class TestJsonFacetRefinement extends SolrTestCaseHS {
             "}"
     );
 
+    // same test, but now the range facet includes "other" buckets
+    // (so we also verify that the "_actual_end" is echoed back)
+    doTestRefine("{top:{type:range, other:all, field:R, start:0, end:1, gap:1, " +
+                 "      facet:{x : {type:terms, field:X, limit:2, refine:true} } } }",
+                 // phase #1
+                 "{top: {buckets:[{val:0, count:2, x:{more:true,buckets:[{val:x1, count:5},{val:x2, count:3}]} } ]," +
+                 "       before:{count:0},after:{count:0}," +
+                 "       between:{count:2,x:{more:true,buckets:[{val:x1, count:5},{val:x2, count:3}]} }," +
+                 "       '_actual_end':'does_not_matter_must_be_echoed_back' } }",
+                 "{top: {buckets:[{val:0, count:1, x:{more:true,buckets:[{val:x2, count:4},{val:x3, count:2}]} } ]," +
+                 "       before:{count:0},after:{count:0}," +
+                 "       between:{count:1,x:{more:true,buckets:[{val:x2, count:4},{val:x3, count:2}]} }," +
+                 "       '_actual_end':'does_not_matter_must_be_echoed_back' } }",
+                 // refinement...
+                 null,
+                 "=={top: {" +
+                 "    _s:[  [0 , {x:{_l:[x1]}} ]  ]," +
+                 "    between:{ x:{_l : [x1]} }," +
+                 "    '_actual_end':'does_not_matter_must_be_echoed_back'" +
+                 "} } ");
+    // imagine that all the nodes we query in phase#1 are running "old" versions of solr that
+    // don't know they are suppose to compute _actual_end ... our merger should not fail or freak out
+    // trust that in the phase#2 refinement request either:
+    //  - the processor will re-compute it (if refine request goes to "new" version of solr)
+    //  - the processor wouldn't know what to do with an _actual_end sent by the merger anyway
+    doTestRefine("{top:{type:range, other:all, field:R, start:0, end:1, gap:1, " +
+                 "      facet:{x : {type:terms, field:X, limit:2, refine:true} } } }",
+                 // phase #1
+                 "{top: {buckets:[{val:0, count:2, x:{more:true,buckets:[{val:x1, count:5},{val:x2, count:3}]} } ]," +
+                 "       before:{count:0},after:{count:0}," +
+                 "       between:{count:2,x:{more:true,buckets:[{val:x1, count:5},{val:x2, count:3}]} }," +
+                 "       } }", // no actual_end
+                 "{top: {buckets:[{val:0, count:1, x:{more:true,buckets:[{val:x2, count:4},{val:x3, count:2}]} } ]," +
+                 "       before:{count:0},after:{count:0}," +
+                 "       between:{count:1,x:{more:true,buckets:[{val:x2, count:4},{val:x3, count:2}]} }," +
+                 "       } }", // no actual_end
+                 // refinement...
+                 null,
+                 "=={top: {" +
+                 "    _s:[  [0 , {x:{_l:[x1]}} ]  ]," +
+                 "    between:{ x:{_l : [x1]} }" + 
+                 "} } ");
+    // a range face w/o any sub facets shouldn't require any refinement
+    doTestRefine("{top:{type:range, other:all, field:R, start:0, end:3, gap:2 } }" +
+                 // phase #1
+                 "{top: {buckets:[{val:0, count:2}, {val:2, count:2}]," +
+                 "       before:{count:3},after:{count:47}," +
+                 "       between:{count:5}," +
+                 "       } }",
+                 "{top: {buckets:[{val:0, count:2}, {val:2, count:19}]," +
+                 "       before:{count:22},after:{count:0}," +
+                 "       between:{count:21}," +
+                 "       } }",
+                 // refinement...
+                 null,
+                 null);
+    
+
     // for testing partial _p, we need a partial facet within a partial facet
     doTestRefine("{top:{type:terms, field:Afield, refine:true, limit:1, facet:{x : {type:terms, field:X, limit:1, refine:true} } } }",
         "{top: {buckets:[{val:'A', count:2, x:{buckets:[{val:x1, count:5},{val:x2, count:3}],more:true} } ],more:true } }",
@@ -380,17 +439,34 @@ public class TestJsonFacetRefinement extends SolrTestCaseHS {
       );
 
       // basic refining test through/under a range facet
-      client.testJQ(params(p, "q", "*:*",
-          "json.facet", "{" +
-              "r1 : { type:range, field:${num_d} start:-20, end:20, gap:40   , facet:{" +
-              "cat0:{${terms} type:terms, field:${cat_s}, sort:'count desc', limit:1, overrequest:0, refine:true}" +
-              "}}" +
-              "}"
-          )
-          , "facets=={ count:8" +
-              ", r1:{ buckets:[{val:-20.0,count:8,  cat0:{buckets:[{val:A,count:4}]}  }]   }" +
-              "}"
-      );
+      for (String end : Arrays.asList(// all of these end+hardened options should produce the same buckets
+                                      "end:20, hardend:true", // evenly divisible so shouldn't matter
+                                      "end:20, hardend:false", "end:20", // defaults to hardened:false
+                                      "end:5, hardend:false", "end:5")) {
+        // since the gap divides the start/end divide eveningly, 
+        // all of these hardend params should we should produce identical results
+        String sub = "cat0:{${terms} type:terms, field:${cat_s}, sort:'count desc', limit:1, overrequest:0, refine:true}";
+
+        // single bucket, all 'other' buckets
+        client.testJQ(params(p, "q", "*:*", "json.facet"
+                             , "{ r1 : { type:range, field:${num_d} other:all, start:-20, gap:40, " + end
+                             + "         , facet:{" + sub + "}}}")
+                      , "facets=={ count:8"
+                      + ", r1:{ buckets:[{val:-20.0,count:8,  cat0:{buckets:[{val:A,count:4}]}  }],"
+                      + "       before:{count:0}, after:{count:0}"
+                      + "       between:{count:8, cat0:{buckets:[{val:A,count:4}]}}"
+                      + "}}");
+        // multiple buckets, only one 'other' buckets
+        client.testJQ(params(p, "q", "*:*", "json.facet"
+                             , "{ r1 : { type:range, field:${num_d} other:between, start:-20, gap:20, " + end
+                             + "         , facet:{" + sub + "}}}")
+                      , "facets=={ count:8"
+                      // NOTE: in both buckets A & B are tied, but index order should break tie
+                      + ", r1:{ buckets:[{val:-20.0, count:4,  cat0:{buckets:[{val:A,count:2}]} },"
+                      + "                {val:  0.0, count:4,  cat0:{buckets:[{val:A,count:2}]} } ],"
+                      + "       between:{count:8, cat0:{buckets:[{val:A,count:4}]}}"
+                      + "}}");
+      }
 
       // test that basic stats work for refinement
       client.testJQ(params(p, "q", "*:*",

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7d8ef9e3/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 ae556bd..fa6a04e 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
@@ -212,6 +212,76 @@ public class TestJsonFacets extends SolrTestCaseHS {
     client.commit();
   }
 
+  /**
+   * whitebox sanity checks that a shard request range facet that returns "between" or "after"
+   * will cause the correct "actual_end" to be returned
+   */
+  public void testRangeOtherWhitebox() throws Exception {
+    Client client = Client.localClient();
+    indexSimple(client);
+
+    // false is default, but randomly check explicit false as well
+    final String nohardend = random().nextBoolean() ? "" : " hardend:false, ";
+    
+    { // first check some "phase #1" requests
+      
+      final SolrParams p = params("q", "*:*", "rows", "0", "isShard", "true", "distrib", "false",
+                                   "_facet_", "{}", "shards.purpose", ""+FacetModule.PURPOSE_GET_JSON_FACETS);
+      final String basic_opts = "type:range, field:num_d, start:-5, end:10, gap:7, ";
+      final String buckets = "buckets:[ {val:-5.0,count:1}, {val:2.0,count:2}, {val:9.0,count:1} ], ";
+      
+      client.testJQ(params(p, "json.facet", "{f:{ " + basic_opts + nohardend + " other:before}}")
+                    , "facets=={count:6, f:{" + buckets
+                    // before doesn't need actual_end
+                    + "   before:{count:1}"
+                    + "} }"
+                    );
+      client.testJQ(params(p, "json.facet", "{f:{" + basic_opts + nohardend + "other:after}}")
+                    , "facets=={count:6, f:{" + buckets
+                    + "   after:{count:0}, _actual_end:'16.0'"
+                    + "} }"
+                    );
+      client.testJQ(params(p, "json.facet", "{f:{ " + basic_opts + nohardend + "other:between}}")
+                    , "facets=={count:6, f:{" + buckets
+                    + "   between:{count:4}, _actual_end:'16.0'"
+                    + "} }"
+                    );
+      client.testJQ(params(p, "json.facet", "{f:{ " + basic_opts + nohardend + "other:all}}")
+                    , "facets=={count:6, f:{" + buckets
+                    + "   before:{count:1},"
+                    + "   after:{count:0},"
+                    + "   between:{count:4},"
+                    + "   _actual_end:'16.0'"
+                    + "} }"
+                    );
+      // with hardend:true, not only do the buckets change, but actual_end should not need to be returned
+      client.testJQ(params(p, "json.facet", "{f:{ " + basic_opts + " hardend:true, other:after}}")
+                    , "facets=={count:6, f:{"
+                    + "   buckets:[ {val:-5.0,count:1}, {val:2.0,count:2}, {val:9.0,count:0} ], "
+                    + "   after:{count:1}"
+                    + "} }"
+                    );
+    }
+
+    { // now check some "phase #2" requests with refinement buckets already specified
+
+      final String facet
+        = "{ top:{ type:range, field:num_i, start:-5, end:5, gap:7," + nohardend
+        + "        other:all, facet:{ x:{ type:terms, field:cat_s, limit:1, refine:true } } } }";
+
+      // the behavior should be the same, regardless of wether we pass actual_end to the shards
+      // because in a "mixed mode" rolling update, the shards should be smart enough to re-compute if
+      // the merging node is running an older version that doesn't send it
+      for (String actual_end : Arrays.asList(", _actual_end:'9'", "")) {
+        client.testJQ(params("q", "*:*", "rows", "0", "isShard", "true", "distrib", "false",
+                             "shards.purpose", ""+FacetModule.PURPOSE_REFINE_JSON_FACETS,
+                             "json.facet", facet,
+                             "_facet_", "{ refine: { top: { between:{ x:{ _l:[B] } }" + actual_end + "} } }")
+                      , "facets=={top:{ buckets:[], between:{x:{buckets:[{val:B,count:3}] }} } }");
+      }
+    }
+  }
+  
   @Test
   public void testExplicitQueryDomain() throws Exception {
     Client client = Client.localClient();
@@ -401,7 +471,8 @@ public class TestJsonFacets extends SolrTestCaseHS {
     // to verify the raw counts/sizes
     assertJQ(req(nestedSKG,
                  // fake an initial shard request
-                 "distrib", "false", "isShard", "true", "_facet_", "{}", "shards.purpose", "2097216")
+                 "distrib", "false", "isShard", "true", "_facet_", "{}",
+                 "shards.purpose", ""+FacetModule.PURPOSE_GET_JSON_FACETS)
              , "facets=={count:5, x:{ buckets:["
              + "   { val:'B', count:3, "
              + "     skg : { "


Mime
View raw message