db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mamta Satoor <msat...@gmail.com>
Subject Re: [VOTE][PATCH]updates on forward only resultset using JDBC 2.0 UpdatableResultset APIs
Date Thu, 31 Mar 2005 04:28:26 GMT
On Wed, 09 Mar 2005 09:34:35 -0800, Daniel John Debrunner
<djd@debrunners.com> wrote:
> 
> +0.8
> 
> I think the functionality is correct, but there are some issues in the
> method checksBeforeUpdateOrDelete(). They could be addressed in a
> subsequent patch.
> 
> - The code at the end of  checksBeforeUpdateOrDelete to get the table
> descriptor and perform column correlation name checks is expensive, and
> seems like it could be done once, rather than on every updateXXX. A
> final solution would ideally address these concerns at compile time and
> not runtime, e.g. part of compilation could be identifying if a
> statement is suitable for updateable result sets.
> 
> - Making method behaviour depend on the name of the calling method is
> not a common programming practise in Derby, and again is expensive. Now
> every updateXXX() call has to perform three String comparisions that are
> not needed. Ie. this code.
> 
>      //the remaining checks only apply to updateXXX methods
> +      if (methodName.equals("updateRow") ||
> methodName.equals("deleteRow") || methodName.equals("cancelRowUpdates"))
> +        return;
> 
> The typical way to address this would be to have the code before this in
> a separate method, have deleteRow, updateRow and cancelRowUpdate methods
> call that new method, and have checksBeforeUpdateOrDelete call that
> method (rather than duplicate code).
> 
> - The lack of synchronization for the updateXXX() methods is also an
> area of concern. As coded two threads performing concurrent updateXXX()
> calls on the same result set will mess each other up, leading to missed
> update values or possibly NullPointerException later if cancelRowUpdates
> is called. But synchronization is a performance hit, and multiple
> threads executing concurrent operations on a JDBC object is not
> recommended. So maybe we can survive without it?
> [Putting the synchronization in getDVDforColumnUpdates() would be the
> logical place, not each updateXXX method.]
> 
> Dan.
> 
> 

I have a patch which takes care of first 2 points raised by Dan. 

Derby does not support correlation name for updatable columns. This
check was earlier made at runtime everytime an updateXXX was issued. I
have moved the check for correlation name to compile time in
CursorNode.java. If the cursor sql has a correlation name for an
updatable column, compile time code will throw an exception. The new
exception code and message has been added to SQLState.java and
messages_en.properties respectively.

The changes in EmbedResultSet.java is to remove the runtime check for
correlation name and to make the various other checks prior to
updateXXX, updateRow and cancelRowUpdates and deleteRow more
efficient.

In addition, I have changed the existing tests to have couple more
checks for compile time catch of correlation name.

Can a commiter please commit this patch for me?

svn stat
M      java\engine\org\apache\derby\impl\sql\compile\CursorNode.java
M      java\engine\org\apache\derby\impl\jdbc\EmbedResultSet.java
M      java\engine\org\apache\derby\iapi\reference\SQLState.java
M      java\engine\org\apache\derby\loc\messages_en.properties
M      java\testing\org\apache\derbyTesting\functionTests\tests\lang\updatableResultSet.java
M      java\testing\org\apache\derbyTesting\functionTests\master\updatableResultSet.out
M      java\testing\org\apache\derbyTesting\functionTests\master\jdk14\updatableResultSet.out

thanks,
Mamta

Mime
View raw message