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