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

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

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

    https://github.com/apache/flink/pull/3330#discussion_r101775264
  
    --- Diff: 
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/codegen/CodeGenerator.scala
 ---
    @@ -1463,21 +1465,23 @@ class CodeGenerator(
         */
       def addReusableFunction(function: UserDefinedFunction): String = {
         val classQualifier = function.getClass.getCanonicalName
    -    val fieldTerm = s"function_${classQualifier.replace('.', '$')}"
    +    val functionSerializedData = serialize(function)
    +    val fieldTerm =
    +      s"""
    +         |function_${classQualifier.replace('.', 
'$')}_${DigestUtils.md5Hex(functionSerializedData)}
    --- End diff --
    
    I think encoding the serialized object as String is a good idea. I like the 
implementation. 
    
    Regarding the name collisions. I did a test with two identical scalar 
functions and everything worked well because `reusableMemberStatements` is a 
hash set that deduplicates the terms.
    
    Btw. what else do we need to do for scalar UDFs? They are injected with the 
same method and hence should be serialized as well. Isn't the only missing 
thing to add more tests? If yes, we could do that in this PR. 
    
    What do you think @sunjincheng121 and @wuchong?


> Improve “UDTF" to support constructor with parameter.
> -----------------------------------------------------
>
>                 Key: FLINK-5795
>                 URL: https://issues.apache.org/jira/browse/FLINK-5795
>             Project: Flink
>          Issue Type: Sub-task
>          Components: Table API & SQL
>            Reporter: sunjincheng
>            Assignee: sunjincheng
>




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

Reply via email to