epotyom commented on code in PR #13702:
URL: https://github.com/apache/lucene/pull/13702#discussion_r1743335144


##########
lucene/facet/src/java/org/apache/lucene/facet/DrillSideways.java:
##########
@@ -414,62 +399,59 @@ public <R> ConcurrentDrillSidewaysResult<R> search(
 
   /**
    * Search using DrillDownQuery with custom collectors. This method can be 
used with any {@link
-   * CollectorOwner}s. It doesn't return anything because it is expected that 
you read results from
-   * provided {@link CollectorOwner}s.
-   *
-   * <p>To read the results, run {@link CollectorOwner#getResult()} for drill 
down and all drill
-   * sideways dimensions.
-   *
-   * <p>Note: use {@link Collections#unmodifiableList(List)} to wrap {@code
-   * drillSidewaysCollectorOwners} to convince compiler that it is safe to use 
List here.
-   *
-   * <p>Use {@link MultiCollectorManager} wrapped by {@link CollectorOwner} to 
collect both hits and
-   * facets for the entire query and/or for drill-sideways dimensions.
+   * CollectorManager}s.
    *
-   * <p>TODO: Class CollectorOwner was created so that we can ignore 
CollectorManager type C,
-   * because we want each dimensions to be able to use their own types. 
Alternatively, we can use
-   * typesafe heterogeneous container and provide CollectorManager type for 
each dimension to this
-   * method? I do like CollectorOwner approach as it seems more intuitive?
+   * <p>Note: Use {@link MultiCollectorManager} to collect both hits and 
facets for the entire query
+   * and/or for drill-sideways dimensions. You can also use it to wrap 
different types of {@link
+   * CollectorManager} for drill-sideways dimensions.
    */
-  public void search(
+  public <C extends Collector, T, K extends Collector, R> Result<T, R> search(
       final DrillDownQuery query,
-      CollectorOwner<?, ?> drillDownCollectorOwner,
-      List<CollectorOwner<?, ?>> drillSidewaysCollectorOwners)
+      CollectorManager<C, T> drillDownCollectorManager,
+      List<CollectorManager<K, R>> drillSidewaysCollectorManagers)
       throws IOException {
-    if (drillDownCollectorOwner == null) {
+    if (drillDownCollectorManager == null) {
       throw new IllegalArgumentException(
           "This search method requires client to provide drill down collector 
manager");
     }
-    if (drillSidewaysCollectorOwners == null) {
+    if (drillSidewaysCollectorManagers == null) {

Review Comment:
   I agree with all points - thanks for reverting the commit.



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

Reply via email to