docs/attachments/LUCENE-4759/LUCENE-4759.patch

Index: lucene/CHANGES.txt
===================================================================
--- lucene/CHANGES.txt	(revision 1444484)
+++ lucene/CHANGES.txt	(working copy)
@@ -69,6 +69,9 @@
 * LUCENE-4750: Convert DrillDown to DrillDownQuery, so you can initialize it
   and add drill-down categories to it. (Michael McCandless, Shai Erera)
 
+* LUCENE-4759: remove FacetRequest.SortBy; result categories are always 
+  sorted by value, while ties are broken by category ordinal. (Shai Erera)
+
 Optimizations
 
 * LUCENE-4687: BloomFilterPostingsFormat now lazily initializes delegate
Index: lucene/facet/src/java/org/apache/lucene/facet/sampling/Sampler.java
===================================================================
--- lucene/facet/src/java/org/apache/lucene/facet/sampling/Sampler.java	(revision 1444484)
+++ lucene/facet/src/java/org/apache/lucene/facet/sampling/Sampler.java	(working copy)
@@ -222,7 +222,6 @@
       setDepth(orig.getDepth());
       setNumLabel(orig.getNumLabel());
       setResultMode(orig.getResultMode());
-      setSortBy(orig.getSortBy());
       setSortOrder(orig.getSortOrder());
     }
     
Index: lucene/facet/src/java/org/apache/lucene/facet/search/DepthOneFacetResultsHandler.java
===================================================================
--- lucene/facet/src/java/org/apache/lucene/facet/search/DepthOneFacetResultsHandler.java	(revision 1444484)
+++ lucene/facet/src/java/org/apache/lucene/facet/search/DepthOneFacetResultsHandler.java	(working copy)
@@ -6,7 +6,6 @@
 import java.util.Collections;
 import java.util.Comparator;
 
-import org.apache.lucene.facet.search.FacetRequest.SortBy;
 import org.apache.lucene.facet.search.FacetRequest.SortOrder;
 import org.apache.lucene.facet.taxonomy.TaxonomyReader;
 import org.apache.lucene.facet.taxonomy.directory.ParallelTaxonomyArrays;
@@ -65,7 +64,6 @@
     assert facetRequest.getDepth() == 1 : "this handler only computes the top-K facets at depth 1";
     assert facetRequest.numResults == facetRequest.getNumLabel() : "this handler always labels all top-K results";
     assert facetRequest.getSortOrder() == SortOrder.DESCENDING : "this handler always sorts results in descending order";
-    assert facetRequest.getSortBy() == SortBy.VALUE : "this handler always sorts results by value";
   }
 
   /** Returnt the value of the requested ordinal. Called once for the result root. */
Index: lucene/facet/src/java/org/apache/lucene/facet/search/FacetRequest.java
===================================================================
--- lucene/facet/src/java/org/apache/lucene/facet/search/FacetRequest.java	(revision 1444484)
+++ lucene/facet/src/java/org/apache/lucene/facet/search/FacetRequest.java	(working copy)
@@ -63,15 +63,6 @@
    */
   public enum FacetArraysSource { INT, FLOAT, BOTH }
 
-  /** Sort options for facet results. */
-  public enum SortBy { 
-    /** sort by category ordinal with the taxonomy */
-    ORDINAL, 
-
-    /** sort by computed category value */ 
-    VALUE 
-  }
-
   /** Requested sort order for the results. */
   public enum SortOrder { ASCENDING, DESCENDING }
 
@@ -82,12 +73,6 @@
   public static final int DEFAULT_DEPTH = 1;
   
   /**
-   * Default sort mode.
-   * @see #getSortBy()
-   */
-  public static final SortBy DEFAULT_SORT_BY = SortBy.VALUE;
-  
-  /**
    * Default result mode
    * @see #getResultMode()
    */
@@ -99,7 +84,6 @@
   private int numLabel;
   private int depth;
   private SortOrder sortOrder;
-  private SortBy sortBy;
 
   /**
    * Computed at construction, this hashCode is based on two final members
@@ -135,7 +119,6 @@
     this.numResults = numResults;
     numLabel = numResults;
     depth = DEFAULT_DEPTH;
-    sortBy = DEFAULT_SORT_BY;
     sortOrder = SortOrder.DESCENDING;
     
     hashCode = categoryPath.hashCode() ^ this.numResults;
@@ -221,11 +204,6 @@
     return resultMode;
   }
 
-  /** Specify how should results be sorted. */
-  public final SortBy getSortBy() {
-    return sortBy;
-  }
-
   /** Return the requested order of results. */
   public final SortOrder getSortOrder() {
     return sortOrder;
@@ -276,10 +254,6 @@
     this.resultMode = resultMode;
   }
 
-  public void setSortBy(SortBy sortBy) {
-    this.sortBy = sortBy;
-  }
-
   public void setSortOrder(SortOrder sortOrder) {
     this.sortOrder = sortOrder;
   }
Index: lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java
===================================================================
--- lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java	(revision 1444484)
+++ lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java	(working copy)
@@ -13,7 +13,6 @@
 import org.apache.lucene.facet.params.CategoryListParams.OrdinalPolicy;
 import org.apache.lucene.facet.search.FacetRequest.FacetArraysSource;
 import org.apache.lucene.facet.search.FacetRequest.ResultMode;
-import org.apache.lucene.facet.search.FacetRequest.SortBy;
 import org.apache.lucene.facet.search.FacetRequest.SortOrder;
 import org.apache.lucene.facet.search.FacetsCollector.MatchingDocs;
 import org.apache.lucene.facet.taxonomy.TaxonomyReader;
@@ -99,7 +98,7 @@
    * {@link FacetRequest}.
    */
   protected FacetResultsHandler createFacetResultsHandler(FacetRequest fr) {
-    if (fr.getDepth() == 1 && fr.getSortOrder() == SortOrder.DESCENDING && fr.getSortBy() == SortBy.VALUE) {
+    if (fr.getDepth() == 1 && fr.getSortOrder() == SortOrder.DESCENDING) {
       FacetArraysSource fas = fr.getFacetArraysSource();
       if (fas == FacetArraysSource.INT) {
         return new IntFacetResultsHandler(taxonomyReader, fr, facetArrays);
Index: lucene/facet/src/java/org/apache/lucene/facet/search/TopKInEachNodeHandler.java
===================================================================
--- lucene/facet/src/java/org/apache/lucene/facet/search/TopKInEachNodeHandler.java	(revision 1444484)
+++ lucene/facet/src/java/org/apache/lucene/facet/search/TopKInEachNodeHandler.java	(working copy)
@@ -38,12 +38,8 @@
  * {@link FacetRequest#categoryPath}, and the enumerated children,
  * {@link FacetResultNode#subResults}, of each node in that {@link FacetResult}
  * are the top K ( = {@link FacetRequest#numResults}) among its children in the
- * taxonomy. Top in the sense {@link FacetRequest#getSortBy()}, which can be by
- * the values aggregated in the count arrays, or by ordinal numbers; also
- * specified is the sort order, {@link FacetRequest#getSortOrder()}, ascending
- * or descending, of these values or ordinals before their top K are selected.
- * The depth (number of levels excluding the root) of the {@link FacetResult}
- * tree is specified by {@link FacetRequest#getDepth()}.
+ * taxonomy. The depth (number of levels excluding the root) of the
+ * {@link FacetResult} tree is specified by {@link FacetRequest#getDepth()}.
  * <p>
  * Because the number of selected children of each node is restricted, and not
  * the overall number of nodes in the {@link FacetResult}, facets not selected
@@ -536,21 +532,10 @@
 
   private ACComparator getSuitableACComparator() {
     if (facetRequest.getSortOrder() == SortOrder.ASCENDING) {
-      switch (facetRequest.getSortBy()) {
-        case VALUE:
-          return new AscValueACComparator();
-        case ORDINAL:
-          return new AscOrdACComparator();
-      }
+      return new AscValueACComparator();
     } else {
-      switch (facetRequest.getSortBy()) {
-        case VALUE:
-          return new DescValueACComparator();
-        case ORDINAL:
-          return new DescOrdACComparator();
-      }
+      return new DescValueACComparator();
     }
-    return null;
   }
 
   /**
@@ -584,26 +569,6 @@
     }
   }
 
-  private static final class AscOrdACComparator extends ACComparator {
-    
-    AscOrdACComparator() { }
-    
-    @Override
-    protected boolean leftGoesNow (int ord1, double val1, int ord2, double val2) {
-      return (ord1 < ord2);
-    }
-  }
-
-  private static final class DescOrdACComparator extends ACComparator {
-    
-    DescOrdACComparator() { }
-    
-    @Override
-    protected boolean leftGoesNow (int ord1, double val1, int ord2, double val2) {
-      return (ord1 > ord2);
-    }
-  }
-
   /**
    * Intermediate result to hold counts from one or more partitions processed
    * thus far. Its main field, constructor parameter <i>mapToAACOs</i>, is a map
Index: lucene/facet/src/java/org/apache/lucene/facet/util/ResultSortUtils.java
===================================================================
--- lucene/facet/src/java/org/apache/lucene/facet/util/ResultSortUtils.java	(revision 1444484)
+++ lucene/facet/src/java/org/apache/lucene/facet/util/ResultSortUtils.java	(working copy)
@@ -49,21 +49,10 @@
     }
 
     if (accending) {
-      switch (facetRequest.getSortBy()) {
-        case VALUE:
-          return new MaxValueHeap(nresults);
-        case ORDINAL:
-          return new MaxOrdinalHeap(nresults);
-      }
+      return new MaxValueHeap(nresults);
     } else {
-      switch (facetRequest.getSortBy()) {
-        case VALUE:
-          return new MinValueHeap(nresults);
-        case ORDINAL:
-          return new MinOrdinalHeap(nresults);
-      }
+      return new MinValueHeap(nresults);
     }
-    throw new IllegalArgumentException("none supported facet request: "+facetRequest);
   }
 
   private static class MinValueHeap extends PriorityQueue<FacetResultNode> implements Heap<FacetResultNode> {
@@ -105,30 +94,6 @@
     }
   }
 
-  private static class MinOrdinalHeap extends PriorityQueue<FacetResultNode> implements Heap<FacetResultNode> {
-    public MinOrdinalHeap(int size) {
-      super(size);
-    }
-
-    @Override
-    protected boolean lessThan(FacetResultNode arg0, FacetResultNode arg1) {
-      return arg0.ordinal < arg1.ordinal;
-    }
-
-  }
-
-  private static class MaxOrdinalHeap extends PriorityQueue<FacetResultNode> implements Heap<FacetResultNode> {
-    public MaxOrdinalHeap(int size) {
-      super(size);
-    }
-
-    @Override
-    protected boolean lessThan(FacetResultNode arg0, FacetResultNode arg1) {
-      return arg0.ordinal > arg1.ordinal;
-    }
-
-  }
-
   /**
    * Create a Heap-Look-Alike, which implements {@link Heap}, but uses a
    * regular <code>ArrayList</code> for holding <b>ALL</b> the objects given,