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-5910) Most CQL3 functions should handle null gracefully
Date Wed, 21 Aug 2013 14:44:51 GMT

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

Sylvain Lebresne commented on CASSANDRA-5910:
---------------------------------------------

bq. I don't think turning RTE into IRE is correct

I don't really disagree and those catch all could probably be removed. The motivation was
that if the execution of a function triggers an unexpected exception, then we send it back
to the user instead of "crashing" server side, but as long as we don't have user custom function,
an unexpected exception in a function is a bug so it's probably fine to throw server side.
And I agree IRE is a bad exception anyway, we just don't have anything better so far.

But the bigger problem is that we have functions (in the selection of a SELECT) that take
their input at execution time. So I don't fully agree with "if it passes validation, it should
execute, and if it doesn't, it's a bug" as that would mean we limit ourselves to functions
that can never error out but that's going to be pretty limiting. We already have the token()
method for which it's very unclear how to deal with null. We could return null on a null parameter,
but if we have a composite partition key (in which case token() is a multi-parameter function),
what to do when only one argument is null and not the other? Of course we could still return
null in that case, but it feels wrong to silent what is essentially an error. Or to take another
example, what do we do about division by 0 if we add a division function tomorrow?

My hunch is that we need a new ExecutionException, though unfortunately it's not that easy
to add new exceptions (rather, there is backward compatibility concerns).

To be clear, we can probably find a simple hack for now, making token() return null as soon
as any of its parameter is null is, while not perfect, probably good enough for now. But it's
probably worth having a longer term plan.

bq. Where does this leave varcharasblob?

varcharasblob (and all the asBlob functions for that matter) execute method just return it's
argument without any processing (it's really just a way to make the CQL type system happy,
since internally everything is already bytes anyway), so if said argument is null, it will
return null.

bq. Wish we'd gone with textasblob

We have it too, we have both. We have one XasBlob and one blobAsX for every CQL3 type. The
only reason varchar is a special case in ByteConversionFcts is that the makeToBlobFunction
method wouldn't generate it.
                
> Most CQL3 functions should handle null gracefully
> -------------------------------------------------
>
>                 Key: CASSANDRA-5910
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-5910
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Sylvain Lebresne
>            Assignee: Sylvain Lebresne
>            Priority: Minor
>             Fix For: 1.2.9
>
>         Attachments: 5910.txt
>
>
> Currently, we don't allow null parameters for functions. So
> {noformat}
> UPDATE test SET d=dateOf(null) WHERE k=0
> {noformat}
> is basically an invalid query. Unfortunately, there's at least one case where we don't
validate correctly, namely if we do:
> {noformat}
> SELECT k, dateOf(t) FROM test
> {noformat}
> In that case, if for any of the row {{t}} is null, we end up with a server side NPE.
But more importantly, throwing an InvalidException in that case would be pretty inconvenient
and actually somewhat wrong since the query is not invalid in itself. So, at least in that
latter case, we want {{dateOf(t) == null}} when {{t == null}}. And if we do that, I suggest
making it always the case (i.e. make the first query valid but assigning {{null}} to {{d}}).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message