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

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

Github user sunjincheng121 commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3993#discussion_r118806216
  
    --- Diff: 
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/functions/utils/UserDefinedFunctionUtils.scala
 ---
    @@ -329,6 +330,21 @@ object UserDefinedFunctionUtils {
       }
     
       /**
    +    * Returns the value (boolean) of AggregateFunction#requiresOver() that 
indicates if this
    +    * AggregateFunction can only be used for over window aggregate. If 
method requiresOver is not
    +    * provided, return false as default value.
    +    */
    +  def getRequiresOverConfig(aggregateFunction: AggregateFunction[_, _]): 
Boolean = {
    +    try {
    +      val method: Method = 
aggregateFunction.getClass.getMethod("requiresOver")
    --- End diff --
    
    I think the method supported to check the `modifiers` as well. Just like 
`checkAndExtractMethods`'s method logic. So I suggest using 
`checkAndExtractMethods` directly.  Changes as follows:
    1. Add boolean param for `checkAndExtractMethods`
    2. Change this method looks like : 
    ```
    val methods = checkAndExtractMethods(aggregateFunction, "requiresOver2", 
true)
         if (methods.isEmpty || methods.length > 1)  {
          return false
        } else {
          methods(0).invoke(aggregateFunction).asInstanceOf[Boolean]
        }
    ```
    In addition, If you want to strictly check, you can check the input 
parameters and return type. What do you think?


> 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