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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
