Thanks Mitesh,
I have a few concerns with this fix.
1. Public API has been changed (getParameter). We can't do that. A new get parameter must be added that takes the third argument.Indicate that it is internal using the INTERNAL: convention in the javadocs
2. The next concern is that type is now used everywhere, and people will expect it to be set all the time. I doubt that it will be. I think that this can be derisked by renaming the variable, or comments.
3. I am honestly quite concerned about the change that will change what we print everytime we bind a string in DB2 and Derby. CAST (? AS VARCHAR(32672)). I would much rather see a change for this Derby work around issolated to Concat. What is the expected impact on CTS? All strings will now be printed this way, is that expected to work?
4. Have you tested this fix with binding off? Will the SQL generated look like: "VARCHAR( 'hi ' || 'there')"
I am wondering if non-bound values support the CAST (...) function? (like this: CAST ('hi' AS VARCHAR(32672)) || CAST ('there' AS VARCHAR(32672). ). If so, then maybe issolating this change to the CONCAT function is better than changing the way EVERY string is bound in DB2 and Derby.
If this is the change, you could simply change the concat expression operator to include the CAST in it and not change all the binding of strings. I realize that your fix opens the door for other CASTable types, but I am not sure that this is the right point to be adding this type of wide sweeping change.
-----Original Message-----
From: Mitesh Meswani [mailto:mitesh.meswani_at_Sun.COM]
Sent: Friday, March 17, 2006 2:54 AM
To: persistence_at_glassfish.dev.java.net
Subject: Fix for issue 400
Attached code fixes issue 400. I still need to do some work/testing
before I can send this out for final review.
Michael,
As you had suspected, the type information of parameter is not
propagated down to ParamaterExpression. I put together a primitive
mechanism to propagate this information.
-ParameterNode#generateExpression now passes getType() to
builder.getParameter
- Expression.getParameter now takes an extra argument "Object type"
which is passed and stored in ParameterExpression
I consume the type in DB2Platform#writeParameterMarker. I am currently
assuming that type is a Class object and compare it to String.Class to
generate appropriate cast. Questions to you:
What are the possible type that a node can take?
Do you see any possible issues/improvements to the type propagation
mechanism above ?
Thanks,
Mitesh