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:34:04 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.
> 
> 

Hi,

I have already submitted a patch for the first 2 comments. As for the
synchronization, I have not added it to avoid performance hit. But if
the community thinks that it is worth taking the performance hit to
avoid multiple threads getting in each others way (keep in mind though
that it is not recommended to have multiple threads on a JDBC object
as Dan pointed out), please send your comments and I can submit the
final patch for this functionality.

thanks,
Mamta

Mime
View raw message