[PATCH] Derby-203 - setNull(x,JDBCType.DATE) does not work when batching is turned on

[Shreyas Kaushik <Sh...@Sun.COM>]

Hi all,

    Please find attached the patch for this. I have updated JIRA with 
the test diffs and test output files.

    With this patch I have attempted to fix the problem that occurs with 
setNull when batching is turned on for all datatypes
where this problem existed. I  had earlier fixed a similar problem for 
Blob and Timestamp only, unknowing that this could occur
elsewhere. This patch I feel address this aspect.


svn stat output is as follows:
------------------------------
M      java/engine/org/apache/derby/iapi/types/SQLDate.java
M      java/engine/org/apache/derby/iapi/types/SQLBinary.java
M      java/engine/org/apache/derby/iapi/types/SQLTime.java
M      
java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/parameterMapping.java
M      
java/testing/org/apache/derbyTesting/functionTests/master/parameterMapping.out


Please review the patch.

~ Shreyas

Re: [PATCH] Derby-203 - setNull(x,JDBCType.DATE) does not work when batching is turned on

[Daniel John Debrunner <dj...@debrunners.com>]

Shreyas Kaushik wrote:

> Hi all,
> 
>    Please find attached the patch for this. I have updated JIRA with the
> test diffs and test output files.
> 
>    With this patch I have attempted to fix the problem that occurs with
> setNull when batching is turned on for all datatypes
> where this problem existed. I  had earlier fixed a similar problem for
> Blob and Timestamp only, unknowing that this could occur
> elsewhere. This patch I feel address this aspect.

> Please review the patch.

Looks good, I'll work on committing it assuming no other review comments.


> +    /** Adding this method to ensure that super class' setInto method doesn't get called
> +      * that leads to the violation of JDBC spec( untyped nulls ) when batching is turned on.
> +      */     
> +    public void setInto(PreparedStatement ps, int position) throws SQLException, StandardException {

Much better job on the comments though in Dan's ideal world there would
be a little more. Things to think about in future comments:

 - Comments often miss the 'why', which can be the essential piece of
information. E.g. a comment such as 'Need to <do something> here.' is
crying out for a 'why', 'Need to <do something> here because <blah>.'
In your comment it's not clear why there is a violation of the JDBC spec.

- To fully understand this comment you need to look at the super-class'
setInto method, but that method may not always exist. Thus if some
cleanup [re]moves that method, this comment becomes much harder to
understand. Thus ensure comments are failrly stand-alone.
[I think the parent setInto method could be moved into UserType]

- Separate out functionality of the method from how it is used, the
previous implementation (using the super's method) was incorrect
regardless of how it was called. Batching was one example. This is
similar to the discussion on the methods Jack is adding for timestampDiff.


Here is an example of the same comment which addresses these points.


/**
Set this DATE value into a PreparedStatement. Uses the explicit
PreparedStatement.setDate() to ensure that the NULL value is passed
in as a null 'typed' as a DATE, implicitly typed by the use of setDate().
The generic PreparedStatement.setObject() cannot be used, as the NULL
value will be passed as an 'untyped' null which violates the JDBC spec
and will cause an exception to be thrown.
Previous use of setObject() in a common setInto method caused batching
to fail with DATE parameters, see Derby 203.
*/

Thanks,
Dan.