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-690) Add scrollable, updatable, insensitive result sets
Date Fri, 10 Mar 2006 11:54:43 GMT
    [ http://issues.apache.org/jira/browse/DERBY-690?page=comments#action_12369848 ] 

Øystein Grøvlen commented on DERBY-690:

My review of the remaining changes to the SQL layer:

5. TableScanResultSet

5.1 General:

    a) I do not understand why you need to prevent re-qualification.
       When will you need to read a row again?  I thought you were
       getting the row from the hash table on later accesses.

    b) Both the write-up and the added comment only talks about
       qualification.  It does not mention anything about
       currentRowIsValid which does seem to address another issue.

5.2 getRowLoaction()/getCurrentRow()

    a) It seems like you have changed the behavior of these functions.
       Should not the JavaDoc be updated to reflect that?  (e.g., when
       null is returned seems to have changed) 

5.3 setRowLocation()

    a) It seems like the comment relates to the testing on isKeyed.
       This relation could have been more obvious.  

    b) Thinking about B-Tree scans: What if a query does not need to
       access the base table only; the index.  (That is, all selected
       columns are part of the index.)  How does the re-positioning
       work in this case?

6. IndexRowToBaseRowResultSet

6.1 positionFromRowLocation

    a) Same question as for TableScanResultSet, when do you need to
       refetch the row from the heap?
    b) I would like a comment that says what concept
       positionFromRowLocation represents.

6.2 getCurrentRow()

    a) The diff would have meen much smaller if you had structured
       this as follows:

       if (positionFromRowLocation) {
           <new code>
           return currentRow;
       <old code>

    b) A comment on why your are testing on positionFromRowLocation
       would be nice.

7. CurrentOfResultSet

7.1 getNextRowCore()

    a) I can not find that these changes is listed in the list of code
       changes in the writeup.

    b) Is skipping the call to target.getCurrentRow() necessary or
       just an optimization?

    c) I do not understand the comment for when to add the warning.
       Why was there not any need for a warning earlier?

    d) Typo: coursor

7.2 updateCachedRow()/markCachedRowAsDeleted()

    a) Isn't the cursor already available as an object attribute?

    b) Another instance of missing parameter name in Javadoc

8. TemporaryRowHolderResultSet

8.1 All new methods: I think the Javadoc should mention that these
    methods are no-ops.

9. NoPutResultSetImpl

9.1 Imports:  Why do you need to import CursorResultSet?

9.2 setCurrentRow()

    a) Why have you removed 'final'?  I cannot find that you have
       added any method that overrides this method.

    b) I guess it makes sense to set its currentRow here, but why was
       it not necessary before?

9.3 updateCachedRow()/markCachedRowAsDeleted()

    a) I think the Javadoc should mention that these methods are

9.4 setRowLocation()

    a) Another instance of missing parameter name in Javadoc

    b) Typo: Positiones

    c) I do not understand the call to targetResultSet.
       targetResultSet is as far as I can tell always an
       InsertResultSet which, according to the javadoc, is used to
       insert rows from a source into a base table.  How is this
       relevant to what you are doing?

10. NormalizeResultSet

10.1 updateCachedRow()
     a) Another instance of missing parameter name in Javadoc

> Add scrollable, updatable, insensitive result sets
> --------------------------------------------------
>          Key: DERBY-690
>          URL: http://issues.apache.org/jira/browse/DERBY-690
>      Project: Derby
>         Type: New Feature
>   Components: JDBC
>     Reporter: Dag H. Wanvik
>     Assignee: Dag H. Wanvik
>     Priority: Minor
>  Attachments: DERBY-690-v1.diff, DERBY-690-v1.stat, DERBY-690-v2.diff, DERBY-690-v2.stat,
SURChanges-v1.pdf, sur-proposal.txt, writeup-v1.html, writeup-v2.html, writeup-v3.html
> JDBC result sets are created with three properties: type, concurrency
> and holdability. The type can be one of TYPE_FORWARD_ONLY,
> be one of CONCUR_READ_ONLY and CONCUR_UPDATABLE. The holdability can
> JDBC allows the full cross product of these. SQL 2003 prohibits the
> combination is supported by some vendors, notably Oracle.
> Currently, Derby supports JDBC result sets in a limited
> way. Holdability is supported. Furthermore, the following is
> supported: 
> 	   - forward-only, read-only 
> 	   - forward-only, updatable (update, delete, but not insert)
> 	     Also, in the network driver, support for some data types
> 	     conversions is missing.
> 	   - scroll insensitive, read-only
> We (Fernanda and Andreas will cooperate with me on this) propose a
> plan to add support for the combination:
> 	   - scroll insensitive, updatable
> for both the embedded driver and the network client driver. 
> As a part of this we would also like to add the missing insert
> operation to the {forward-only, updatable} result sets (JIRA-100), and
> remove the requirement for an explicit "FOR UPDATE" clause in the SQL
> query to achieve updatability if CONCUR_UPDATABLE is specified
> (JIRA-231).
> The full proposal text is uploaded as an attachment, including a proposed
> functional specification.
> This JIRA will  be used to track sub-issues for this effort. The sub-issues will be linked
back to this issue.

This message is automatically generated by JIRA.
If you think it was sent incorrectly contact one of the administrators:
For more information on JIRA, see:

View raw message