Comments inline
Pramod Gopinath wrote:
>Hi Tom
> Thanks for your quick reply. My comments are inline.
>
>Tom Ware wrote:
>
>
>>Hi Guys,
>>
>> A couple of comments.
>>
>>1. Why do we need the parameterBound variable in
>>DatabasePlatform.setComplexParameterValue(). Can we not just replace
>>the following:
>>
>> parameterBound = false;
>> }
>> return parameterBound;
>>
>>with the following:
>>
>> return false;
>> }
>> return true;
>>
>>
>>
>Instead of having multiple return, we prefer to set a boolean value and
>do a return at the end of the method. This is more of our coding
>preference.
>But personally we do not have anything against one approach or the
>other. If you prefer the above style could change the code to that.
>
>
The TopLink coding style tends to be the one I mention. If you feel
strongly about your choice, I will not object, but prefer the strategy
with the two returns.
>
>
>>2. In PostGreSQLPlatform is it possible to change the "instance of"
>>calls to use our getJDBCType() method. For an example of its' use,
>>look in DatabasePlatform.setComplexParameterValue(). That method
>>should be faster than "instance of"
>>
>>
>>
>I was taking a look at the DatabasePlatform.getJDBCType() and had query
>for you. Lets say I have a Long paramater and want to do a setLong(),
>this will not work. The getJDBCType() would return Types.INTEGER and
>the code would then only do a setInt(). Isn't this wrong ?
>Moreover the code as currently written has been tested extensively on
>the cmp 2.x code base. It has not shown up as a hotpoint in an of the
>performance profiling activities. Hence we are more comfortable with the
>code.
>
>
>
You make an excellent point. Go ahead and continue with the "instance
of" strategy.
-Tom
>>3. The following code need an else clause in case we get a Number that
>>is not currently addressed. It should call statement.setObject(index,
>>parameter);
>>
>> Number number = (Number) parameter;
>> if (number instanceof Integer) {
>> statement.setInt(index, number.intValue());
>> } else if (number instanceof Long) {
>> statement.setLong(index, number.longValue());
>> } else if (number instanceof Short) {
>> statement.setShort(index, number.shortValue());
>> } else if (number instanceof Byte) {
>> statement.setByte(index, number.byteValue());
>> } else if (number instanceof Double) {
>> statement.setDouble(index, number.doubleValue());
>> } else if (number instanceof Float) {
>> statement.setFloat(index, number.floatValue());
>> } else if (number instanceof BigDecimal) {
>> statement.setBigDecimal(index, (BigDecimal) number);
>> } else if (number instanceof BigInteger) {
>> statement.setBigDecimal(index, new
>>BigDecimal((BigInteger) number));
>> }
>>
>>
>>
>OK.
>
>
>>4. I would prefer to keep this change in PostGreSQLPlatform. There
>>are several reasons for this.
>>- So far we have only seen this issue on PostGreSQL
>>- To change this on DatabasePlatform, we will have to run quite a
>>large number of certification tests for the various other databases.
>>- In the worst case scenario, we have added 9 comparison operations to
>>determine what jdbc call to make. Without a large number of databases
>>showing this error, adding this number of comparisons to the base
>>class is overkill.
>>
>>
>OK.
>
>
>>-Tom
>>
>>Pramod Gopinath wrote:
>>
>>
>>
>>>Hi Tom/Gordon
>>> In case of Postgresql, when doing an insert into a field that
>>>corresponds to a Byte type, we get an exception thrown from the jdbc
>>>driver. I have attached the complete stack trace to this email.
>>>The issue was because we were doing a setObject(index,
>>>parameterValue) and Postgresql jdbc driver complains that you need to
>>>use the setObject(index,parameterValue, Types)
>>>
>>>These are the changes made to address this problem :
>>>
>>>src/java/oracle/toplink/essentials/internal/databaseaccess/DatabasePlatform.setParameterValueInDatabaseCall()
>>>
>>> refactored code and introduced 2 methods :
>>>setComplexParameterValue() & setPrimitiveParameterValue()
>>> Other than the refactoring of code there is no code changes made
>>>to this file.
>>>
>>>src/java/oracle/toplink/essentials/platform/database/PostgreSQLPlatform.setPrimitiveParameterValue()
>>>
>>> this method is overridden to use reflection on the parameter
>>>value and delegate the call to the appropriate setter method of the
>>>prepared statement. This code is identical to the code being used in
>>>the exiting cmp code.
>>> We think that this code should ideally be in the
>>>DatabasePlatform, as there might be other drivers that have the same
>>>issue. What do you guys think about this ?
>>>
>>>I have attached the exception stack, diffs and code to this email. If
>>>you guys want to discuss this over a concall we are free tomorrow
>>>after 10:00AM PST.
>>>
>>>Thanks
>>>Pramod & Mitesh
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>
>
>