gsmiller commented on a change in pull request #159:
URL: https://github.com/apache/lucene/pull/159#discussion_r641859710



##########
File path: lucene/facet/src/java/org/apache/lucene/facet/DrillSideways.java
##########
@@ -335,10 +342,22 @@ protected boolean scoreSubDocsAtOnce() {
     /** Hits. */
     public final TopDocs hits;
 
+    /** Collector to compute Facets */
+    FacetsCollector drillDowns;
+
+    /** drill-down dimensions */
+    Map<String, Integer> drillDownDims;

Review comment:
       Great question! Both of these are reasonable suggestions with different 
"ripple effects" I think. Changing the method signature of `buildFacetResult` 
is possible, but is non-backwards compatible. So there would be some users 
impacted when upgrading if they've subclassed `DrillSideways` with this method 
overridden. I thought of putting a `Map` in the response since it seemed like a 
"friendlier" API for users to work with, but maybe that's a silly thought since 
`buildFacetsResult` is also effectively a public API. I think it's totally 
valid to be consistent with the `buildFacetResult` method signature here and 
provide parallel arrays for the dimensions and associated `FacetsCollector`s. I 
would suggest adding some nice javadoc to those fields describing them (which I 
would recommend regardless of the approach I suppose). Thanks again!




-- 
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.

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

Reply via email to