[ 
https://issues.apache.org/jira/browse/FLINK-6725?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16027148#comment-16027148
 ] 

ASF GitHub Bot commented on FLINK-6725:
---------------------------------------

Github user shaoxuan-wang commented on the issue:

    https://github.com/apache/flink/pull/3993
  
    Thanks for the review @fhueske @sunjincheng121 . 
    Fabian, yes, I do not think this will be a blocker for release. In fact, 
this is a similar interface clean-up issue as FLINK-6457 that we want to solve. 
But certainly, we can just merge this to master and include this in the next 
release.
    
    Jincheng, yes, it is one's flavor about how to implement an interface with 
contracted method. From my point of view:
    1. A contract method is usually to be very useful as a flag to indicate the 
characteristic of an aggregator, and this information will help the optimizer 
to decide the plan. Such methods are retract/merge/resetAccumulator.
    2. I prefer we should just expose the user interface without any default 
implementation (an interface method with default implementation will anyway not 
control the user behavior). The user defined interface IMO should purely ask 
users to provide minimum required methods that can let the UDX work. 
    3. We will call getAccumulatorType and getResultType also during the 
compilation to generate the plan I think. If we put requiresOver into the 
interface, we should also consider to put getAccumulatorType and getResultType 
into interface as well.
    
    Also, I think we may want to provide all the agg that requiresOver as the 
built-in agg in the future. Thereby, we may not need to expose this interface 
in the UDAGG at that point.


> make requiresOver as a contracted method in udagg
> -------------------------------------------------
>
>                 Key: FLINK-6725
>                 URL: https://issues.apache.org/jira/browse/FLINK-6725
>             Project: Flink
>          Issue Type: Improvement
>          Components: Table API & SQL
>            Reporter: Shaoxuan Wang
>            Assignee: Shaoxuan Wang
>
> I realized requiresOver is defined in the udagg interface when I wrote up the 
> udagg doc. I would like to put requiresOver as a contract method. This makes 
> the entire udagg interface consistently and clean.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to