cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-7809) UDF cleanups (#7395 follow-up)
Date Thu, 21 Aug 2014 13:56:10 GMT

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

Sylvain Lebresne commented on CASSANDRA-7809:
---------------------------------------------

Pushed [branch|https://github.com/pcmanus/cassandra/commits/7809] with the change suggested
above. The branch has a bunch of commits described below:
# 'Introduce FunctionName class': the code passes the namespace and name of a function all
the time, which is a bit annoying and forces concatening them for every error message. Not
a huge deal but using a FunctionName class is slightly cleaner imo (it's also consitent with
the UTName we use for UDT).
# 'Make Functions handle overload of a function with different return types': slightly generalize
the code in {{Functions}} so that the different overloads of a function don't have to have
the same return type.
# 'Special case Token function and remove Function.Factory': the {{token}} function is somewhat
special in that it's argument types actually depend on the table it's applied to since they
depend on the partition key definition. The code was using {{Function.Factory}} to handle
this, but it's overkill and was complicating the following commits so this commit change it
to instead special case the {{token}} method, which is much simpler overall.
# 'Merge UDF code with existing function code': this is the main commit. It refactors the
code so that both native and UDF functions share as much code as possible, and it fixes most
of the issues described above.
# 'Prefer exact type match when resolving functions': the code in Functions was detecting
if multiple overloads were applicable for a given call-site (because of a bind marker for
instance), but it wasn't properly breaking the ambiguity if there was an exact type match.
The commit fixes that.
# 'Update test': mainly adds new tests to exercise the bugs describe above.


> 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
>              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)

Mime
View raw message