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 2004/12/08 15:36:16 UTC

Re: [VOTE][PATCH]delete from the resultset using JDBC 2.0 Updatable Resultset APIs

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mamta Satoor wrote:

> Hi,
>
> Now that we have branched the codeline for a stable release, it looks
> like this might be a good time to come back to the patch for
> JDBC 2.0 Updatable Resultset apis - delete functionality only.
> Since this is a new feature, it was decided on the list to wait until
> a stable release to review and checkin the patch.
>
> I will start out with my +1 vote :-). Please send in your votes.

It would be useful with complex changes like this if contributors
included the 'svn status' output relevant to the change. Makes it easier
to see the scope of the changes.


Questions

- - You are using the flag 'rowGotUpdatedDeleted' to indicate the current
row has been deleted. Then you use that flag in ResultSet.rowDeleted(),
but that method in JDBC 1.4.1 Javadoc says it indicates *a* row in the
ResultSet has been deleted, not just the current row. While I believe
your implementation is more logical, we have to follow the spec. :-( Is
this a mistake in the Javadoc and clarified in the tutorial book? (I
don't have mine handy at the moment).

- - When setting the 'hole' row, your comment says it sets values to
0/NULL, but the code always does a setNull(). But, the real question is
what is meant to happen when the ResultSetMetaData indicates the column
is not nullable?

- - After ResultSet.deleteRow() is called, is it specified anywhere what
the position of the ResultSet is? E.g. remains on the 'hole' row, is
before the next row?

- - Also any specification for calling deleteRow() on an already deleted
row? Your implementation disallows this.
[these last three questions are closely related]

- - The use of a positioned delete SQL statement to implement the
deleteRow() works and thus allows Derby to support the functionality
required, but I'm concerned it will be slow. I wonder if the code could
not create the runtime objects itself, rather than going through the
compiler. I see why it was done this way, and especially when you come
to insertRow() and updateRow(), using SQL allows a quick implementation
without a lot of repeated tricky code. The use of SQL may be the best
way to get updateable result sets into Derby and the matching tests.

- - I didn't check the tests, but a test that ensured delete triggers were
fired on a deleteRow() would be essential.


Some comments on the proposed changes


These new SQLStates have the SQLStandard warning first two characters of
'01', but are used as SQLExceptions.

+    String UPDATABLE_RESULTSET_API_DIALLOWED = "01J06";
+    String UPDATABLE_RESULTSET_API_DIALLOWED_ON_A_HOLE = "01J07";


In the text of the error messages it is probably best to use 'ResultSet'
when you mean a java.sql.ResultSet and not 'resultset'.

Dan.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFBtxFEIv0S4qsbfuQRAkdhAKDXKhMtNGwnEreeQ7aBkm1CFxj43ACgjlhK
RleYoz4Yf7m79anI0zbhPbU=
=hiej
-----END PGP SIGNATURE-----


Re: [VOTE][PATCH]delete from the resultset using JDBC 2.0 UpdatableResultsetAPIs

Posted by Daniel John Debrunner <dj...@debrunners.com>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mamta Satoor wrote:


> I thought further about the 3 issues that we have been discussing.
> 2a)We can continue to make the columns that are not nullable as null but
> since it is just a hole, a program should not be using the contents of the
> deleted hole anyways. And the program can find out from rowDeleted
> if they are on a deleted hole.
>
> 2b)Just as with positioned deletes, the ResultSet can be positioned right
> before the next row (as Dan suggested in his alternative approach).

For the forward result set, moving off the row after a delete row is the
same as not detecting deletes, as the ResultSet can never be positioned
on the deleted row.

I think this is the correct approach.

I think allowing any of the getXXX() methods to return NULL when the
column's metadata says it is not nullable is trouble. We have seen
issues in the past when Cloudscape had a system table column incorrectly
labelled as not-nullable and it contained nulls, which were then
returned from a ResultSet.

In the future I think when/if Derby does support 'holes' in scrollable
updateable result sets, then an application can use rowDeleted() to see
if the row is a 'hole' (has been deleted), but any getXXX() call on such
a row should throw an exception.

Dan.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFBvgwNIv0S4qsbfuQRAnyDAJ9pZeFaHTNhoPJdwh3u23a0bjHiBwCfTWsA
bKLAD07zGpI8KT5R2pGgAXc=
=V1a9
-----END PGP SIGNATURE-----


Re: [VOTE][PATCH]delete from the resultset using JDBC 2.0 UpdatableResultsetAPIs

Posted by Mamta Satoor <ma...@Remulak.Net>.

Daniel John Debrunner wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Mamta Satoor wrote:
>
> >
> > Hi Dan,
> >
> > Thanks for taking the time to do the review. Answers to your questions
> > 1)The tutorial book says that ResultSet.rowDeleted determines whether *the
> > current row* in this ResultSet object has been deleted. Going on this
> > information, I will leave my code for rowDeleted as is. But if this seems
> > incorrect to anyone, please let me know.
> >
> > 2)As for the next set of 3 questions,
> > a)If ResultSetMetaData indicates that one/more columns are not
> nullable, then
> > what should happen to those columns when the driver is trying to make a
> > delete row hole with null values for all the columns in the row?
> > b)Where should be ResultSet positioned after a successful deleteRow call?
> > c)What should happen if deleteRow is called more than once on the same
> row?
> >
> > I haven't found anything in the JDBC docs or in the Tutorial book about
> > what a JDBC driver should do in above sceanrios. I would appreciate
> > any feedback from the community on these 3 questions.
>
> An alternate approach would be to return false for
> DatabaseMetaData.deletesAreDetected().
>
> Then:
>         1) becomes a non-issue.
>         2a) becomes a non-issue
>         2b) the result set would be positioned before the next row
>         2c) can't happen.
>
> I think deletesAreDetected() returning true is only useful for
> scrollable result sets, which you are not supporting. And the
> deletesAreDetected() method does take a result set type, so in the
> future scrollable results sets could support holes, while forward only
> did not.
>
> Any reason you selected to detect deletes?
>
> Dan.

Before we explore the alternative approach, here are some tidbits about the
implementation that I submitted.

The reason I chose to detect deletes is that to me, it seemed like the complete
implementation of delete rather than just saying deletes are not detected.
And, it was easy to implement :-)

I thought further about the 3 issues that we have been discussing.
2a)We can continue to make the columns that are not nullable as null but
since it is just a hole, a program should not be using the contents of the
deleted hole anyways. And the program can find out from rowDeleted
if they are on a deleted hole.

2b)Just as with positioned deletes, the ResultSet can be positioned right
before the next row (as Dan suggested in his alternative approach).

2c)In my implementation, I am letting the driver code throw an exception if the
second deleteRow is attempted on a deleted row. Rather than that, I think
it will be better to let the engine deal with the second deleteRow attempt.
Then we will get the same exception as for positioned delete. eg from Derby's
existing positioned delete support (this is what updatable resultsets rely on)
ij> -- .target cursor on already deleted row
get cursor c9 as 'select * from t1 for update';
ij> next c9;
I          |V         |D                     |T
------------------------------------------------------
1          |1111111111|1.1E12                |11:11:11
ij> delete from t1 where current of c9;
1 row inserted/updated/deleted
ij> delete from t1 where current of c9;
ERROR XCL08: Cursor 'C9' is not on a row.

If the community agrees to this, I can submit another patch with these changes
and others suggested by Dan for SQLState change, error message fix and test
for delete trigger.

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:

>
> Hi Dan,
>
> Thanks for taking the time to do the review. Answers to your questions
> 1)The tutorial book says that ResultSet.rowDeleted determines whether *the
> current row* in this ResultSet object has been deleted. Going on this
> information, I will leave my code for rowDeleted as is. But if this seems
> incorrect to anyone, please let me know.
>
> 2)As for the next set of 3 questions,
> a)If ResultSetMetaData indicates that one/more columns are not
nullable, then
> what should happen to those columns when the driver is trying to make a
> delete row hole with null values for all the columns in the row?
> b)Where should be ResultSet positioned after a successful deleteRow call?
> c)What should happen if deleteRow is called more than once on the same
row?
>
> I haven't found anything in the JDBC docs or in the Tutorial book about
> what a JDBC driver should do in above sceanrios. I would appreciate
> any feedback from the community on these 3 questions.

An alternate approach would be to return false for
DatabaseMetaData.deletesAreDetected().

Then:
	1) becomes a non-issue.
	2a) becomes a non-issue
	2b) the result set would be positioned before the next row
	2c) can't happen.

I think deletesAreDetected() returning true is only useful for
scrollable result sets, which you are not supporting. And the
deletesAreDetected() method does take a result set type, so in the
future scrollable results sets could support holes, while forward only
did not.

Any reason you selected to detect deletes?

Dan.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFBuicyIv0S4qsbfuQRAiSIAJwN2Hxf9NvvPmn3VMRD27XvEN/I/gCZAemA
MrlA7KER8TBpgJ9qmQcxXHM=
=nI/C
-----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
>
> Mamta Satoor wrote:
>
> > Hi,
> >
> > Now that we have branched the codeline for a stable release, it looks
> > like this might be a good time to come back to the patch for
> > JDBC 2.0 Updatable Resultset apis - delete functionality only.
> > Since this is a new feature, it was decided on the list to wait until
> > a stable release to review and checkin the patch.
> >
> > I will start out with my +1 vote :-). Please send in your votes.
>
> It would be useful with complex changes like this if contributors
> included the 'svn status' output relevant to the change. Makes it easier
> to see the scope of the changes.
>
> Questions
>
> - - You are using the flag 'rowGotUpdatedDeleted' to indicate the current
> row has been deleted. Then you use that flag in ResultSet.rowDeleted(),
> but that method in JDBC 1.4.1 Javadoc says it indicates *a* row in the
> ResultSet has been deleted, not just the current row. While I believe
> your implementation is more logical, we have to follow the spec. :-( Is
> this a mistake in the Javadoc and clarified in the tutorial book? (I
> don't have mine handy at the moment).
>
> - - When setting the 'hole' row, your comment says it sets values to
> 0/NULL, but the code always does a setNull(). But, the real question is
> what is meant to happen when the ResultSetMetaData indicates the column
> is not nullable?
>
> - - After ResultSet.deleteRow() is called, is it specified anywhere what
> the position of the ResultSet is? E.g. remains on the 'hole' row, is
> before the next row?
>
> - - Also any specification for calling deleteRow() on an already deleted
> row? Your implementation disallows this.
> [these last three questions are closely related]
>
> - - The use of a positioned delete SQL statement to implement the
> deleteRow() works and thus allows Derby to support the functionality
> required, but I'm concerned it will be slow. I wonder if the code could
> not create the runtime objects itself, rather than going through the
> compiler. I see why it was done this way, and especially when you come
> to insertRow() and updateRow(), using SQL allows a quick implementation
> without a lot of repeated tricky code. The use of SQL may be the best
> way to get updateable result sets into Derby and the matching tests.
>
> - - I didn't check the tests, but a test that ensured delete triggers were
> fired on a deleteRow() would be essential.
>
> Some comments on the proposed changes
>
> These new SQLStates have the SQLStandard warning first two characters of
> '01', but are used as SQLExceptions.
>
> +    String UPDATABLE_RESULTSET_API_DIALLOWED = "01J06";
> +    String UPDATABLE_RESULTSET_API_DIALLOWED_ON_A_HOLE = "01J07";
>
> In the text of the error messages it is probably best to use 'ResultSet'
> when you mean a java.sql.ResultSet and not 'resultset'.
>
> Dan.
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.2.5 (MingW32)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iD8DBQFBtxFEIv0S4qsbfuQRAkdhAKDXKhMtNGwnEreeQ7aBkm1CFxj43ACgjlhK
> RleYoz4Yf7m79anI0zbhPbU=
> =hiej
> -----END PGP SIGNATURE-----

Hi Dan,

Thanks for taking the time to do the review. Answers to your questions
1)The tutorial book says that ResultSet.rowDeleted determines whether *the
current row* in this ResultSet object has been deleted. Going on this
information, I will leave my code for rowDeleted as is. But if this seems
incorrect to anyone, please let me know.

2)As for the next set of 3 questions,
a)If ResultSetMetaData indicates that one/more columns are not nullable, then
what should happen to those columns when the driver is trying to make a
delete row hole with null values for all the columns in the row?
b)Where should be ResultSet positioned after a successful deleteRow call?
c)What should happen if deleteRow is called more than once on the same row?

I haven't found anything in the JDBC docs or in the Tutorial book about
what a JDBC driver should do in above sceanrios. I would appreciate
any feedback from the community on these 3 questions.

Also, I am thinking that as a first stab at quick implementation of this
functionality, we can go through the compiler rather than going directly to
runtime objects. We can then later come back to performance improvement
of this feature.

I added test for referential integrity enforcement when a row is deleted
using JDBC api but didn't add a test for trigger. Will do so.

thanks,
Mamta