db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "A B (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-1620) SQL CASE statement returns ERROR 42X89 when including NULL as a return value
Date Tue, 05 Jun 2007 22:52:26 GMT

    [ https://issues.apache.org/jira/browse/DERBY-1620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12501730
] 

A B commented on DERBY-1620:
----------------------------

Hi John,

Thank you for following through with my previous review comments, and for rewriting your test
cases to run in JUnit.  That was very helpful.

Just a few minor comments on the latest patch:

1 - The Java class name at the top of the license header in the new JUnit test says "NullIfTest",
when it should say "CaseExpressionTest".

2 - There are a few unnecessary imports in CaseExpressionTest:

  java.sql.Date
  java.sql.PreparedStatement
  java.sql.Time
  java.sql.Timestamp

  org.apache.derbyTesting.junit.JDBC

3 - In the "suite()" method I think it might be good to use existing convenience methods on
TestConfiguration, instead of calling "baseSuite" directly.  Ex:

  // For embedded:

  suite.addTest(
      TestConfiguration.embeddedSuite(CaseExpressionTest.class));

  // For client/server:

  suite.addTest(
      TestConfiguration.clientServerSuite(CaseExpressionTest.class));

  // For both embedded *and* client/server:

  suite.addTest(
      TestConfiguration.defaultSuite(CaseExpressionTest.class));

That said, since the changes for Jira only effect SQL processing within the engine, it's probably
good enough to just run the new JUnit test in embedded mode.

4 - There are several System.out.printlns in the test.  I think that the preferred approach
to JUnit testing is to avoid printing anything to System.out or System.err.  If the output
is an important part of the test then is should be replaced with some kind of JUnit assertion.
 But in the new CaseExpressionTest, it looks like the System.out.println statements are purely
informational, in which case I think it's best to remove them altogether.  Or, if you think
the output might be useful for debugging, you could move all of the System.outs into a "debug"
method and add a flag that allows debugging information to be turned on/off (with default
to "off").  See, for example, lang/MathTrigFunctionsTest.java.

5 - I think the new test has to be added to lang/_Suite.java in order to run as part of Derby's
JUnit regression suite.  This should just be a one-line addition to the "suite()" method of
lang/_Suite.java, something like:

    suite.addTest(CaseExpressionTest.suite());

6 - It might be nice if you can name your next patch something other than "ConditionalNode.diff",
in order to avoid confusion with the changes that have already been committed.  For example,
something like "derby1620_test.patch" would, I think, be a tad more clear.

Thanks again for your continued work (and patience) with this Jira!  I think that if the above
comments can be addressed, the patch will be ready for commit and we can finally close this
issue...

> 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
>            Assignee: John Peterson
>            Priority: Minor
>             Fix For: 10.3.0.0
>
>         Attachments: ConditionalNode.diff, 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.


Mime
View raw message