Hi Dan, Thanks for taking the time to go through my mail and your input. I have added a Jira entry DERBY-2776 for all the work required other than item 2 which you are tackling as an independent Jira.
Mamta On 6/7/07, Daniel John Debrunner <[EMAIL PROTECTED]> wrote:
Mamta Satoor wrote: > One other solution could be that the setting of CAST node's collation > type to current compilation schema's collation type can be moved out of > CastNode.bindCastNodeOnly() method and into CastNode.bindExpression (). > I checked through Derby code for internally generated CAST nodes and > noticed that except for ConditionalNode, everywhere else, after the CAST > node is created, we call CastNode.bindCastNodeOnly() method on it. For > some unknown reason, ConditionalNode doesn't call just > CastNode.bindCastNodeOnly() but instead calls CastNode.bindExpression(). > So, the complete fix to the problem could be to have ConditionalNode > call CastNode.bindCastNodeOnly() instead of CastNode.bindExpression() > and the collation type setting moved into CastNode.bindExpression () from > CastNode.bindCastNodeOnly(). I think there are two issues here: 1) It might be cleaner with the above solution to also have an explicit boolean field in CastNode that indicates if the CAST is internal or not. The use of different methods (as above) probably works, but those current method names don't imply to me the behaviour you are implementing. So there's some chance in the future that a new call may break the assumptions. Having explicit code would be clear and easy to understand. 2) I think that modifying a DTD to change its collation may be pushing the envelope of "bug-free" code. Now, as you have seen, if several ValueNode's all refer to a single DTD then changing the collation of one changes all of them. This is probably not what is required. The setCollation() pattern did copy the existing setNullablity() pattern which I think is probably also wrong and subject to bugs. Changing DTD to be (logically) immutable would be a much better solution, e.g. DTD.setNullability() returns a new DTD with the nullability set correctly, leaving the old one unmodified. (The method should also have a name change, setXXX() is wrong since it is not setting the state of the object it is called on). (Also such a method can return the same object if the state is unchanged). I will enter a Jira for this(immutable DTD) and work on it, I also noticed the other day that the type handling in some ValueNodes is inconsistent, one node I think has three methods that return its type (and probably don't all return the same value :-). Dan.
