db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel John Debrunner <...@apache.org>
Subject Re: Collation info of internally generated CAST node
Date Thu, 07 Jun 2007 15:14:29 GMT
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.

Mime
View raw message