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

Tyler Hobbs commented on CASSANDRA-7809:
----------------------------------------

This is pretty close to being good to go.

Some nitpicks:
* FunctionCall.java: "Cannot assign result of function..." should use 
receiver.name() in message
* Functions.java: "none of its type signature matches" should be "none of its 
type signatures match"
* NativeFunction.java: unused import of java.util.List
* AbstractFunction.java: unused import of java.util.Arrays
* CreateFunctionStatement.java: unused imports
* DropFunctionStatement.java: unused import of Schema
* UFTest.java: unused import, should put "stopForcingPreparedValues()" in a 
finally block

It also looks like we don't have any unit or dtest coverage for type casts 
(besides the few you added in UFTest).  Can you add unit tests here or open a 
ticket for that?  We also don't document typecasting in the CQL3 docs.

> UDF cleanups (#7395 follow-up)
> ------------------------------
>
>                 Key: CASSANDRA-7809
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-7809
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Sylvain Lebresne
>            Assignee: Sylvain Lebresne
>              Labels: cql
>             Fix For: 3.0
>
>
> The current code for UDF is largely not reusing the pre-existing 
> mechanics/code for native/hardcoded functions. I don't see a good reason for 
> that but I do see downsides: it's more code to maintain and makes it much 
> easier to have inconsitent behavior between hard-coded and user-defined 
> function. More concretely, {{UDFRegistery/UDFFunctionOverloads}} 
> fundamentally do the same thing than {{Functions}}, we should just merge 
> both. I'm also not sure there is a need for both {{UFMetadata}} and 
> {{UDFunction}} since {{UFMetadata}} really only store infos on a given 
> function (contrarly to what the javadoc pretends).  I suggest we consolidate 
> all this to cleanup the code, but also as a way to fix 2 problems that the 
> UDF code has but that the existing code for "native" functions don't:
> * if there is multiple overloads of a function, the UDF code picks the first 
> version whose argument types are compatible with the concrete arguments 
> provided. This is broken for bind markers: we don't know the type of markers 
> and so the first function match may not at all be what the user want. The 
> only sensible choice is to detect that type of ambiguity and reject the 
> query, asking the user to explicitly type-cast their bind marker (which is 
> what the code for hard-coded function does).
> * the UDF code builds a function signature using the CQL type names of the 
> arguments and use that to distinguish multiple overrides in the schema. This 
> means in particular that {{f(v text)}} and {{f(v varchar)}} are considered 
> distinct, which is wrong since CQL considers {{varchar}} as a simple alias of 
> {{text}}. And in fact, the function resolution does consider them aliases 
> leading to seemingly broken behavior.
> There is a few other small problems that I'm proposing to fix while doing 
> this cleanup:
> * Function creation only use the function name when checking if the function 
> exists, which is not enough since we allow multiple over-loadings. You can 
> bypass the check by using "OR REPLACE" but that's obviously broken.
> * {{IF NOT EXISTS}} for function creation is broken.
> * The code allows to replace a function (with {{OR REPLACE}}) by a new 
> function with an incompatible return type. Imo that's dodgy and we should 
> refuse it (users can still drop and re-create the method if they really want).



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to