[ 
https://issues.apache.org/jira/browse/BEAM-5892?focusedWorklogId=160104&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-160104
 ]

ASF GitHub Bot logged work on BEAM-5892:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 29/Oct/18 17:55
            Start Date: 29/Oct/18 17:55
    Worklog Time Spent: 10m 
      Work Description: akedin commented on a change in pull request #6865: 
[BEAM-5892] Improve UDF registration
URL: https://github.com/apache/beam/pull/6865#discussion_r229035334
 
 

 ##########
 File path: 
sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/BeamSqlDslUdfUdafTest.java
 ##########
 @@ -233,10 +257,18 @@ public Integer addInput(Integer accumulator, Integer 
input) {
   }
 
   /** An example UDF for test. */
-  public static class CubicInteger implements BeamSqlUdf {
+  public static class Cubic implements BeamSqlUdf {
     public static Integer eval(Integer input) {
       return input * input * input;
     }
+
+    public static Long eval(Long input) {
 
 Review comment:
   I don't think I like this approach to solve this problem. Few reasons:
   * the output of these functions are different. Sure, they represent the same 
math function, but int/float/double types are different, so end results can be 
different (e.g. comparisons like `eval(num1) eq eval(num2)` can behave 
unexpectedly if the users are not aware of the details);
   * there is nothing to enforce that even the same math function is 
implemented by all functions, or that edge cases are handled the same way. It 
is very easy to introduce bugs when modifying non-trivial functions;
   * it is completely invisible to the users, they don't know when one function 
is invoked vs another, harder to debug;
   * it is hard to find out what overloads are implemented (e.g. why no Decimal 
support in this case);
   * SQL users might not know about the implementation details and to them it 
may be magic why behavior is not what they expect;
   
   I think there are better approaches to solve similar problems:
    * add explicit casts to the plan if the input type doesn't match the arg 
type:
      * it is visible in the plan;
      * if the cast fails it is visible in the exception;
      * no extra code from the users for simple cases like this;
      * no magic;
    * implement UDFs as instances, not just static method calls:
      * support generics;
      * support configuration;
      * support inheritance;

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 160104)
    Time Spent: 0.5h  (was: 20m)

> Allow registering UDF with the same method name but different argument list
> ---------------------------------------------------------------------------
>
>                 Key: BEAM-5892
>                 URL: https://issues.apache.org/jira/browse/BEAM-5892
>             Project: Beam
>          Issue Type: Improvement
>          Components: dsl-sql
>            Reporter: Rui Wang
>            Assignee: Rui Wang
>            Priority: Major
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to