|
Yip Ng made changes - 28/Oct/06 07:31 PM
Yip Ng made changes - 29/Oct/06 11:10 PM
Yip Ng made changes - 29/Oct/06 11:13 PM
Yip Ng made changes - 29/Oct/06 11:14 PM
The change to ConstantNode is fine. Thanks for finding and fixing this. On adding tests for this bug: Would it be better to add it to the JUnit test (GroupByExpression.java) rather than the old canon style test?
I'm not sure what kind of equivalence isEquivalent, but is the following correct?
+ // value can be null which represents a SQL NULL value. + return ( (other.getValue() == null && getValue() == null) || I'm always ware wrt. comparision of SQL NULL values, since NULL = whatever (and any other comparision operator) will evaluate to UNKNOWN, not TRUE. Thanks Manish and Bernt for reviewing the patch.
Bernt: Good question. If I am not mistaken, the isEquivalent() method is used to compare the select column against the group by with expression. Note that it is comparing the structural form of the two expressions for equivalence at bind phase and not comparing the actual row values at runtime to produce a result. Thus, I believe in this context, it is valid. Manish: I'll see if I can find some time to convert the testcases in the patch to JUnit testcases. Thanks for the suggestion. The behaviour of isEquivalent() is described by the javadoc comments for ValueNode.isEquivalent().
Bernt, do you think the description needs to be improved in some way? It's vital to get the javadoc for such a method to be accurate and clear. Dan is this the Javadoc you are referring to?
http://db.apache.org/derby/javadoc/engine/org/apache/derby/impl/sql/compile/ValueNode.html#isEquivalent(org.apache.derby.impl.sql.compile.ValueNode) If so, then yes, I think the description needs to be improved. I think that the topics that need to be described include: - equivalency of basic data types, including "interesting" data types such as floating point and date types - behavior of NULL in equivalency checks - equivalency of expressions Yip, do you think you can propose some candidate Javadoc for us to review?
Yip Ng made changes - 31/Oct/06 05:58 PM
Attaching patch derby2014-trunk-diff02.txt for
Yip Ng made changes - 03/Nov/06 12:42 AM
Yip Ng made changes - 03/Nov/06 12:42 AM
Thanks Yip! The new Javadoc is clear and detailed and very useful.
The new JUnit tests are also very clear and easy to follow. The only little nit I have is that they seem to mix tabs and spaces. Sorry about the mix of tabs and space, I was using 2 different editors. Submitting patch derby2014-trunk-diff03.txt. This patch fixes the mixed tab/spaces and uses 80 character column margin for the updated javadoc for ValueNode.isEquivalent() method. Please review.
Yip Ng made changes - 04/Nov/06 04:48 AM
I'm ready to commit this, but I wanted to confirm the Javadoc change and am having trouble running 'ant derbydocs', even on a clean trunk. I sent mail to the developer list asking for help.
Andrew fixed the problem I was having with the derbydocs build (thanks Andrew!)
I added a few paragraph and list tags to the ValueNode.isEquivalent javadoc to make it a bit more friendly in the HTML display and committed the 03 patch to subversion as revision 471459.
Bryan Pendleton made changes - 05/Nov/06 04:47 PM
Ported to 10.2 branch at subversion revision 480306.
Rick Hillegas made changes - 29/Nov/06 12:36 AM
This issue has been resolved for over a year with no further movement. Closing.
Andrew McIntyre made changes - 13/Dec/07 09:05 AM
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Closed
Major
NullPointerException with NULLIF in GROUP BY clause
DERBY-2014. Again the NPE happens in isEquivalent() method where it does not handle value is null. (same symptom asDERBY-2008) and the patch addresses this + additonal testcases. derbyall + junit suite passes.