[GitHub] [lucene] Yuti-G commented on a change in pull request #747: LUCENE-10325: Add getTopDims functionality to Facets

[GitBox]

Yuti-G commented on a change in pull request #747:
URL: https://github.com/apache/lucene/pull/747#discussion_r827269196



##########
File path: 
lucene/facet/src/java/org/apache/lucene/facet/sortedset/SortedSetDocValuesFacetCounts.java
##########
@@ -190,20 +235,45 @@ private FacetResult getPathResult(
       String[] parts = FacetsConfig.stringToPath(term.utf8ToString());
       labelValues[i] = new LabelAndValue(parts[parts.length - 1], 
ordAndValue.value);
     }
+    return labelValues;
+  }
 
-    if (dimConfig.hierarchical == false) {
+  /** Returns value/count of a dimension. */
+  private Number getDimValue(

Review comment:
       Thanks @gsmiller! The reason I wanted to return `Number` is that 
FacetResult takes dimCount in Number as a parameter, but it can take `int` as 
well. 
   
   > /** Sole constructor. */
     public FacetResult(
         String dim, String[] path, Number value, LabelAndValue[] labelValues, 
int childCount) {
       this.dim = dim;
       this.path = path;
       this.value = value;
       this.labelValues = labelValues;
       this.childCount = childCount;
     }
   
   I can change the return type from `Number` to `int`. The only concern is 
that it would change the following behavior when `q == null` since I was trying 
to be consistent with the original `getPathResult` function. However, I just 
tested changing the return type from `Number` to `int` in my local and still 
passed all the tests.
   
   <img width="421" alt="Screen Shot 2022-03-15 at 10 56 33 AM" 
src="https://user-images.githubusercontent.com/44444710/158441533-ecf1bb90-eb47-4fef-850f-75a934441040.png";>
   
   <img width="711" alt="Screen Shot 2022-03-15 at 10 55 33 AM" 
src="https://user-images.githubusercontent.com/44444710/158441360-10cb8869-a1df-4787-a11b-22a225478bc2.png";>
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org