You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by Daniel John Debrunner <dj...@debrunners.com> on 2005/01/04 20:24:48 UTC
Re: [VOTE][PATCH]delete from the resultset using JDBC 2.0 Updatable
Resultset APIs
-----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.
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.
deleteRow() method
Why is the check on theResults.isClosed() needed?
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.
A comment for the line 'rowData = null' would be very useful
SQLState.java
- --------------
Typo in constant, need S in DISALLOWED
UPDATABLE_RESULTSET_API_DIALLOWED
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-----
Re: [VOTE][PATCH]delete from the resultset using JDBC 2.0 UpdatableResultset
APIs
Posted by Edward Rayl <er...@bellsouth.net>.
>When the Statement object is created with CONCUR_UPDATABLE, the
>ResultSet object will return concurrency as CONCUR_UPDATABLE if the
>underlying language result set is updateable. If the underlying language resultset
>is not updatable, there should be a warning issued on the ResultSet object that this
>is not an updateable ResultSet. But, what should the ResultSet object return for
>concurrency in this case? I think it should return CONCUR_READ_ONLY.
>And any attempts to updateable resultset apis will throw exceptions as well
>since the ResultSet object is not updateable.
>
>
In the case of Oracle, it would return CONCUR_READ_ONLY. I would the
imagine a JDBC programmer would expect this behavior from Derby as well.
Re: [VOTE][PATCH]delete from the resultset using JDBC 2.0
UpdatableResultset APIs
Posted by Mamta Satoor <ma...@Remulak.Net>.
Daniel John Debrunner wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> >>>>
> 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.
I want to make sure that I understand the right behavior of this method.
When the Statement object is created with CONCUR_READ_ONLY,
ResultSet object created on it should return CONCUR_READ_ONLY
(that is the FOR UPDATE clause should not decided the cuncurrency
of the ResultSet object).
When the Statement object is created with CONCUR_UPDATABLE, the
ResultSet object will return concurrency as CONCUR_UPDATABLE if the
underlying language result set is updateable. If the underlying language resultset
is not updatable, there should be a warning issued on the ResultSet object that this
is not an updateable ResultSet. But, what should the ResultSet object return for
concurrency in this case? I think it should return CONCUR_READ_ONLY.
And any attempts to updateable resultset apis will throw exceptions as well
since the ResultSet object is not updateable.
I will add tests to test for the concurrency of the ResultSet and
Statement objects once we have decided on the correct behavior.
thanks,
Mamta
Re: [VOTE][PATCH]delete from the resultset using JDBC 2.0 UpdatableResultset
APIs
Posted by Daniel John Debrunner <dj...@debrunners.com>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Mamta Satoor wrote:
>
>
> 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
Thanks for making the changes, especially the comments.
I'll work on getting it committed.
Dan.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFB3vtXIv0S4qsbfuQRAuWtAJ0cLcwfSeJ89gFi4EBpZLPl/X3jywCfTXjo
9cYOOT7wVuGiBdsVJi2yVaU=
=1Etf
-----END PGP SIGNATURE-----
Re: [VOTE][PATCH]delete from the resultset using JDBC 2.0
UpdatableResultset APIs
Posted by Mamta Satoor <ma...@Remulak.Net>.
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