Github user shaoxuan-wang commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3735#discussion_r111986331
  
    --- Diff: 
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/aggregate/GeneratedAggregations.scala
 ---
    @@ -36,6 +36,15 @@ abstract class GeneratedAggregations extends Function {
       def setAggregationResults(accumulators: Row, output: Row)
     
       /**
    +    * Calculates the results from accumulators, and set the results to the 
output (with key offset)
    +    *
    +    * @param accumulators the accumulators (saved in a row) which contains 
the current
    +    *                     aggregated results
    +    * @param output       output results collected in a row
    +    */
    +  def setAggregationResultsWithKeyOffset(accumulators: Row, output: Row)
    --- End diff --
    
    This sounds a very good idea. I actually have thought to merge 
*WithKeyOffset functions into the existing functions. It works for most 
functions, but `setAggregationResults` and `setAggregationResults` are a little 
tricky. For `accumulate` and `setAggregateResults`, they do not need keyOffset, 
but for `merge`, they need.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to