[
https://issues.apache.org/jira/browse/DERBY-1620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
A B updated DERBY-1620:
-----------------------
Derby Info: (was: [Patch Available])
Fix Version/s: 10.3.0.0
Assignee: (was: John Peterson)
Hi John, just a few minor comments on the most recent patch:
- The following two methods do not have any javadoc; it's not that big of a
deal since it's pretty clear that what they are doing--but a line or two
of javadoc content wouldn't hurt...
* isCastNode(ValueNode)
* isCastToChar(CastNode)
- There are several un-used versions of "shouldCast()" in this patch: the
only one that is directly called is (DataTypeDescriptor, ValueNode), and
that simply calls the version which takes two DTDs. So I wonder if we could
just have the single version that takes two DTDs and then call that
directly, ex:
Instead of:
if (isNullNode(thenNode) && shouldCast(castType, thenNode)) {
just do:
if (isNullNode(thenNode) &&
shouldCast(castType, thenNode.getTypeServices()))
{
Then you could remove all of the other versions of the method.
- The sequential "if" statements in the new "findType()" method are a tad
hard to follow. If we remove the underlying code blocks we end up with
something like:
+ if ((thenType != null) && !isCastNode(thenNode) &&
!isConditionalNode(thenNode))
+ if (isCastNode(thenNode) && !isCastToChar((CastNode)thenNode))
+ if ((elseType != null) && !isCastNode(elseNode) &&
!isConditionalNode(elseNode))
+ if (isCastNode(elseNode) && !isCastToChar((CastNode)elseNode))
+ if (isConditionalNode(thenNode))
+ if (theType != null)
+ if (isConditionalNode(elseNode))
+ if (theType != null)
Maybe I'm just being paranoid, but when I see code like this I have to
stare for a long time to convince myself that all of the different paths
have been covered. It's especially tough given that certain conditions (ex.
"isCastNode(thenNode)") show up more than once.
Of course, you added good comments to this code which were very helpful--so
thank you for that! I don't think you need to change anything here, but
since
I spent a good deal of time scratching my head and trying to convince myself
that all of this is correct, I thought I'd mention it...
But these are all minor things that can be addressed with a follow-up patch if
you are so inclined.
I ran derbyall and suites.All on Red Hat Linux with ibm142 and there were no
failures, so I made some minor formatting changes to the patch (lines longer
than 80 chars--it'd be great if these could be avoided in the future...) and
committed the *engine* changes with svn #520173:
URL: http://svn.apache.org/viewvc?view=rev&rev=520173
Since you have kindly agreed to work on rewriting the tests for JUnit (thank
you!) I did not commit the "resultset" changes and am leaving the issue 'Open'
for now...
Thank you for the contribution!
> SQL CASE statement returns ERROR 42X89 when including NULL as a return value
> ----------------------------------------------------------------------------
>
> Key: DERBY-1620
> URL: https://issues.apache.org/jira/browse/DERBY-1620
> Project: Derby
> Issue Type: Bug
> Components: SQL
> Affects Versions: 10.2.1.6
> Environment: Windows XP
> Reporter: John Peterson
> Priority: Minor
> Fix For: 10.3.0.0
>
> Attachments: ConditionalNode.diff, ConditionalNode.diff,
> ConditionalNode.diff, Derby_Community_Discussion.doc, derbyall_report.txt,
> resultset.tmp, resultset.tmp, sysinfo_and_example.txt
>
>
> This bug appears to be related to the DERBY-7 bug (NULLIF() function). When
> NULL is used during a CASE statement, Derby requires the NULL to be CAST to
> the appropriate type. This does not appear to meet the SQL 2003 Standard for
> the Case Expression (see attached Word document). See the attached Word
> document to view the Derby Community Discussion about this issue. See the
> attached .TXT to view the SYSINFO and to see an example of the steps to
> reproduce using IJ.
> Steps to Reproduce:
> ij>values case when 1=2 then 3 else NULL end;
> ERROR 42X89: Types 'INTEGER' and 'CHAR' are not type compatible. Neither
> type is assignable to the other type.
> Current Workaround:
> ij>values case when 1=2 then 3 else cast(NULL as INT) end;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
