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