-----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-----
