[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_r831558514



##########
File path: 
lucene/facet/src/java/org/apache/lucene/facet/sortedset/SortedSetDocValuesFacetCounts.java
##########
@@ -178,10 +229,23 @@ private FacetResult getPathResult(
       }
     }
 
-    if (q == null) {
-      return null;
+    if (dimConfig.hierarchical == false) {

Review comment:
       Thanks @gsmiller! I added the code below in getDimValue to exit the 
function earlier if these conditions have been met.
   `
   if (dimConfig.hierarchical == true || (dimConfig.multiValued && 
dimConfig.requireDimCount)) {
         return counts[dimOrd];
    }
   `
   But if I add the same conditions to `getChildOrdsResult`, it would be a 
duplicate check when calling getChildOrdsResult from `getDimValue`. 
   
   `GetPathResult` is another method that calls `getChildOrdsResult`, and I 
think the first condition in the return statement plus the existing check in 
`getChildOrdsResult` handles all the conditions. Please correct me if I am 
wrong. Thanks again! :)
   
   <img width="851" alt="Screen Shot 2022-03-21 at 2 17 52 PM" 
src="https://user-images.githubusercontent.com/44444710/159365382-52d86138-4092-4aec-a811-f41c579c64fb.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