db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Øystein Grøvlen (JIRA) <derby-...@db.apache.org>
Subject [jira] Commented: (DERBY-775) Network client: Add support for scrollable, updatable, insensitive result sets
Date Fri, 07 Apr 2006 10:07:24 GMT
    [ http://issues.apache.org/jira/browse/DERBY-775?page=comments#action_12373613 ] 

Øystein Grøvlen commented on DERBY-775:
---------------------------------------

Changes looks good.  Just a few follow-up comments/questions:

Dag H. Wanvik (JIRA) wrote:
>>>>>> "Øystein" == Øystein Grøvlen (JIRA) <derby-dev@db.apache.org>
wrote:
> Øystein> 1. metadata_net.properties:
> Øystein> 
> Øystein> - Is the format and constant values used here documented anywhere?
> 
> As part of DERBY-965, I added some comments explaining the syntax of
> the fields I touched, please see the file from line 197 onwards. I
> am not aware of any other documentation.

What about the constants like 1004.  How did you figure out which
values to use?

> Øystein> 4. NetCursor
> 
> Øystein> - Why do you have to delay the call to setIsUpdataDeleteHole until
> Øystein>   later and not do it when testing for warnings?
> 
> The existing code checked for update/delete holes by relying only
> nulldata, which is parsed later than the warning. For SUR, in addition
> to nulldata, we send a warning (SQLState.ROW_DELETED) since this is
> required by DRDA, cf the write-up section on this. The existing code
> is probably non-compliant. Rather than calling setIsUpdataDeleteHole()
> in two places, we chose to use the existing code location for calling
> it, using the helper variable receivedDeleteHoleWarning in the SUR
> case. I agree the code could be simplified by removing the old logic
> (relying only on nulldata), but we chose the conservative approach.

It might be conservative, but it is not very cautious.  It seems like
you silently ignore scenarios where you get both a warning and data,
and the case where you get no data and no warning.  Should not these
scenarios generate an error or at least be handled by an assert?

> 
> Øystein> 
> Øystein> 6. Statement
> Øystein> 
> Øystein> - Why have you made warnings_ protected?  It seems like it is package
> Øystein>   private that is needed.  Why not keep it private and supply a
> Øystein>   get-method?
> 
> Agreed, done. The old code is too not good on encapsulation, so it
> leads one astray ;-) Introduced a protected method
> Statement#getWarnings_, an accessor for warnings_, which is now
> private.

'_'-suffix for method names. Is that common?


> Øystein> - relativex(): Why do not the isBeforeFirstX()/isAfterLastX() test for
> Øystein>   all result sets?
> 
> For SUR, the check is performed by the getAbsoluteRowset. 

Is there a particular reason why this code cannot be common to all
result sets?

> Øystein> - updateRow(): I tried to browse the spec and the javadoc and I did
> Øystein>   not find anything that said that there is a difference
> Øystein>   between forward only cursors and scrollable cursors with respect to
> Øystein>   validity of the current position after and update.  Where is this
> Øystein>   stated?
> 
> This is not mandated by JDBC. I remember asking Dan about it once, but
> I don't rememeber the rationale for making forward only cursors lose
> the position after an updateRow. I checked the SQL 2003 standard just
> now, but I could not find it is a requirement for positioned update,
> either.

To me it seems like a common behavior for forward-only and scrollable
cursors should be more important than a specific behavior for any of
them.

> 
> Øystein> 
> Øystein> - checkForUpdatableResultSet(): What is the difference between this
> Øystein>   new method and the existing checkUpdatableCursor()?  It seems better
> Øystein>   to change the existing than to make a new method.
> 
> Merged these methods into one and updated masters to match the changed
> (and better) exceptions.

Good.  I see there are few other places that could have used this
method (insertRow(), refreshRow(), checkUpdatePreconditions().  Either
you could change that, or we should send a note to David to make him
aware of it for his internationalization work.


> Network client: Add support for scrollable, updatable, insensitive result sets
> ------------------------------------------------------------------------------
>
>          Key: DERBY-775
>          URL: http://issues.apache.org/jira/browse/DERBY-775
>      Project: Derby
>         Type: New Feature

>   Components: Network Client, JDBC
>     Versions: 10.2.0.0
>     Reporter: Dag H. Wanvik
>     Assignee: Dag H. Wanvik
>     Priority: Minor
>  Attachments: 775-writeup.txt, derby-775-1.diff, derby-775-1.stat, derby-775-2.diff,
derby-775-2.stat, derby-775-3.diff, derby-775-3.stat
>
> This is a part of the DERBY-690 effort.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


Mime
View raw message