[
https://issues.apache.org/jira/browse/PHOENIX-3918?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16043672#comment-16043672
]
James Taylor commented on PHOENIX-3918:
---------------------------------------
Thanks for the updated patch, [~tdsilva]. It's good that we identify built-in
functions that potentially are not handling null correctly. The patch doesn't
look right, though. Let me try to explain the difference between returning
false versus true and handling null:
- a function should only return false if any child expressions return false.
This means "I don't have enough information to calculate a result". This can
really only happen when executing on the server side during filter evaluation.
In this case, the expression evaluation is only seeing partial state:
essentially each Cell is fed into the expression and an attempt is made to
evaluate it. For example {{WHERE A + B < 5}} might see the Cell for A first,
but not yet have seen B, so false would be returned for the + expression and
subsequently by the < expression. Once B is seen, then the expression can be
evaluated.
- in the case that a child returns true, it may have been evaluated to null.
This is the case when ptr.getLength() == 0. When a child returns a value, it
will always return the same same value, so there's no need to continue
evaluating it again and again. There are compound expressions such as AND and
OR that take advantage of this. If false is returned, though, these compound
expressions would be evaluated again and again. So this code isn't correct:
{code}
- if (!offsetExpr.evaluate(tuple, ptr)) return false;
+ if (!offsetExpr.evaluate(tuple, ptr) || ptr.getLength() == 0)
return false;
{code}
- For most (but not all) built-in functions, when null is encountered (i.e. a
child expression evaluated, returning true, and ptr.getLength()==0), then the
function can immediately return null. So most often, the code will look like
this:
{code}
if (!offsetExpr.evaluate(tuple, ptr)) return false;
if (ptr.getLength() == 0) return true;
{code}
- The exception are expressions like || and ARRAY_CAT which combines children
together simply skipping null children.
> Ensure all function implementations handle null args correctly
> --------------------------------------------------------------
>
> Key: PHOENIX-3918
> URL: https://issues.apache.org/jira/browse/PHOENIX-3918
> Project: Phoenix
> Issue Type: Bug
> Reporter: Samarth Jain
> Assignee: Thomas D'Silva
> Fix For: 4.12.0
>
> Attachments: PHOENIX-3918.patch, PHOENIX-3918-v2.patch
>
>
> {code}
> testBothParametersNull(org.apache.phoenix.end2end.TimezoneOffsetFunctionIT)
> Time elapsed: 2.272 sec <<< ERROR!
> java.sql.SQLException: ERROR 201 (22000): Illegal data. Unknown timezone
> at
> org.apache.phoenix.end2end.TimezoneOffsetFunctionIT.testBothParametersNull(TimezoneOffsetFunctionIT.java:130)
> timezoneParameterNull(org.apache.phoenix.end2end.TimezoneOffsetFunctionIT)
> Time elapsed: 2.273 sec <<< ERROR!
> java.sql.SQLException: ERROR 201 (22000): Illegal data. Unknown timezone
> at
> org.apache.phoenix.end2end.TimezoneOffsetFunctionIT.timezoneParameterNull(TimezoneOffsetFunctionIT.java:151)
> dateParameterNull(org.apache.phoenix.end2end.TimezoneOffsetFunctionIT) Time
> elapsed: 2.254 sec <<< ERROR!
> java.sql.SQLException: ERROR 201 (22000): Illegal data. Expected length of at
> least 8 bytes, but had 0
> at
> org.apache.phoenix.end2end.TimezoneOffsetFunctionIT.dateParameterNull(TimezoneOffsetFunctionIT.java:172)
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
