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.
