db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mamta Satoor <ma...@Remulak.Net>
Subject Re: [VOTE][PATCH]delete from the resultset using JDBC 2.0 UpdatableResultset APIs
Date Fri, 07 Jan 2005 20:35:31 GMT


Daniel John Debrunner wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> > I would like to resubmit the patch for Updatable ResultSet JDBC 2.0
> > - delete functionality only. This patch has changes as discussed in thread
> >
> http://nagoya.apache.org/eyebrowse/ReadMsg?listName=derby-dev@db.apache.org&msgNo=1356
> >
> > My vote : +1. Please send in your votes.
>
> Some comments:
>
> EmbedDatabaseMetaData
> - ---------------------
>
> Can you explain changing the other updates are visible methods to
> false, maybe by comments in the code.
>

After further going through the JDBC api and tutorial book, forward only
resultsets can see the changes made by others. This is because Derby
materializes the forward only resultset incrementally. And hence, the other
updates are visible methods should continue to return true for forward only
resultsets. But scroll insensitive resultsets by their definition are immune to
changes made by others and hence for scroll insensitive resultsets, we should
return false. I have changed the code accordingly.

>
> EmbedResultSet20
> - ----------------
>
> getConcurrency() method
>     I think this implementation is incorrect. A ResultSet that is
> created read-only, but uses a FOR UPDATE clause will return it is
> updateable. This is currently used for positioned updates.
> I think also a ResultSet that is created as updateable will change to be
> read only if the underlying language result set is not updateable. In
> this case I think a warning should be generated as well, which I don't
> think will happen with your code.
>

ResultSet object will return CONCUR_READ_ONLY if Statement object
has that concurrency set on it. But if the Statement object has
CONCUR_UPDATABLE, then ResultSet object will return the same if
the underlying language resultset object is updatable. If not, ResultSet
object will return CONCUR_READ_ONLY.

>
>  deleteRow() method
>     Why is the check on theResults.isClosed() needed?
>

In case of autocommit on, if there was an exception which caused
runtime rollback prior to this deleteRow, then the rollback code will
mark the language resultset closed (it doesn't mark the JDBC ResultSet closed).
That is why alongwith the earlier checkIfClosed call in this method, there
is a check for language resultset close as well.(Existing Derby code does the
same 2 checks for navigational methods like next() on the ResultSet object).

>
>     When pushing the statement context, shouldn't you use the text of
> the delete SQL statement, and not the sql text of the current result
> set? Also you are passing in the parameters of the query which are not
> required and not relevant to the positioned delete.
>

I have fixed this.

>
>     A comment for the line 'rowData = null' would be very useful
>

Added comment.

>
> SQLState.java
> - --------------
>     Typo in constant, need S in DISALLOWED
>     UPDATABLE_RESULTSET_API_DIALLOWED
>

Fixed the typo.

>
> Dan.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.2.5 (MingW32)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iD8DBQFB2u2AIv0S4qsbfuQRAu6nAKC18s2fPxUDJEfif8Htktrx7QoRKwCfWzSu
> ABOTO+fQyqEwHz5V/UBRzL8=
> =IAO9
> -----END PGP SIGNATURE-----

Attached is the new patch with all the feedback so far. Following is the svn stat o/p
M      java\engine\org\apache\derby\impl\jdbc\EmbedConnectionContext.java
M      java\engine\org\apache\derby\impl\jdbc\EmbedResultSet.java
M      java\engine\org\apache\derby\impl\jdbc\EmbedDatabaseMetaData.java
M      java\engine\org\apache\derby\impl\jdbc\EmbedResultSet20.java
M      java\engine\org\apache\derby\impl\jdbc\EmbedConnection.java
M      java\engine\org\apache\derby\iapi\sql\ResultSet.java
M      java\engine\org\apache\derby\iapi\reference\SQLState.java
M      java\engine\org\apache\derby\jdbc\Driver169.java
M      java\engine\org\apache\derby\jdbc\Driver20.java
M      java\engine\org\apache\derby\loc\messages_en.properties
A      java\testing\org\apache\derbyTesting\functionTests\tests\lang\updatableResultSet.java
A      java\testing\org\apache\derbyTesting\functionTests\master\updatableResultSet.out
M      java\testing\org\apache\derbyTesting\functionTests\suites\derbylang.runall


thanks,
Mamta

Mime
View raw message