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


##########
pinot-common/src/main/java/org/apache/pinot/common/response/BrokerResponse.java:
##########
@@ -123,6 +127,17 @@ String toJsonString()
    */
   long getMinConsumingFreshnessTimeMs();
 
+  /**
+   * Get the max number of rows seen by a single operator in the query 
processing chain.
+   * <p>
+   * In single-stage this value is equal to {@link 
#getNumEntriesScannedPostFilter()} because single-stage operators
+   * cannot generate more rows than the number of rows received. This is not 
the case in multi-stage operators, where
+   * operators like join or union can emit more rows than the ones received on 
each of its children operators.
+   */
+  default long getMaxRowsInOperator() {

Review Comment:
   This is not a new method but a method that existed in `BrokerResponseNative` 
and was inherited by `BrokerResponseV2` . Given it is already there (even it 
doesn't make sense) and it was used to calculate `isPartialResponse` I think it 
is useful to have it in the base interface now that `BrokerResponseNativeV2` 
does not inherit `BrokerResponseNative` anymore.
   
   The alternative would be to do not make `isPartialResponse` `default` and 
include the same code in both `BrokerResponseNative` and 
`BrokerResponseNativeV2`. If you think that is better I can change it, but it 
looks a bit repetitive. For sure currently how this stats are being declared 
doesn't make sense, but we can try to improve that in the future



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

Reply via email to