gortiz commented on code in PR #8979:
URL: https://github.com/apache/pinot/pull/8979#discussion_r977481626


##########
pinot-core/src/main/java/org/apache/pinot/core/operator/combine/BaseCombineOperator.java:
##########
@@ -250,6 +250,18 @@ protected boolean isQuerySatisfied(T resultsBlock) {
    */
   protected abstract void mergeResultsBlocks(T mergedBlock, T blockToMerge);
 
+  /**
+   * Converts the given generic IntermediateResultBlock into the specific type 
of IntermediateResultBlock that is
+   * supported by this combine operator.
+   *
+   * This operator may return the same object if the runtime type of the given 
block already matches which what it is
+   * expected.
+   *
+   * The returned object will usually be the one received as first argument of
+   * {@link #mergeResultsBlocks(BaseResultsBlock, BaseResultsBlock)}.
+   */
+  protected abstract T createInitialResultBlock(BaseResultsBlock block);

Review Comment:
   I don't think we should use T here because it may be the case that the child 
operator returns a different block type than the one used in the 
BaseCombineOperator. By adding T here we:
   - Need to cast the child operator, whose static type is usually Block or 
BaseResultBlock, to T before calling this method.
   - Couple the child operator and the combine operator when it is not needed.
   
   I've just changed the code a little bit make this explicit by adding another 
method such as:
   
   ```java
     /**
      * Converts the given generic BaseResultsBlock into the specific type of 
BaseResultsBlock that is supported by this
      * operator.
      *
      * This operator may return the same object if the runtime type of the 
given block already matches which what it is
      * expected.
      *
      * The returned object will usually be the one received as second argument 
of
      * {@link #mergeResultsBlocks(T, T)}
      */
     protected T convertToAppendableBlock(BaseResultsBlock block) {
       return (T) block;
     }
   ```



-- 
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: commits-unsubscr...@pinot.apache.org

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


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

Reply via email to