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.
> 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.
> 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
>>
>>
>>
>>
>>
>