[ 
http://issues.apache.org/jira/browse/DERBY-690?page=comments#action_12370159 ] 

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

More review comments.  This time for the changes to the access layer:

11. ScanController

11.1 fetchWithoutQualify()

     a) I think the Javadoc should be more verbose and at least list
        parameters and contain "@see fetch".  I also think it would
        have been nice to be a bit more verbose on what not applying
        qualifiers mean.

     b) I would have placed this new method below fetch

11.2 positionAtRowLocation()

     a) Why can not clients use the existing reopenScanByRowLocation
        instead?

12. GenericScanController

12.1 reopenScanByRecordHandleAndSetLocks()

     a) If I have understood things correct, when a scan is initially
        opened, the first row is not locked.  Locking happen on the
        subsequent next().  Why could not a similar scheme be used
        here? That is, reopen positions just before the specified row
        and a subsequent call to next is performed to actually lock
        it.  Looking at fetchRows() and the methods it calls, there
        seems to already exist code to handle a repositioned scan.
        (The combination of SCAN_INIT and a set record posisiton).

     b) I am still a bit uncomfortable with the method names in the
        following call path: 
            setRowLocation() 
                positionAtRowLocation()
                    reopenScanByRecordHandleAndSetLocks().  
        The lower levels makes more happen than the names of the high
        level methods indicates.

13. Scan/SortBufferRowSource/SortScan

13.1 Javadoc

     a) Javadoc: I think you should at least put a sentence about what
        the methods does in addition to referring to the methods of
        ScanController.  At least for some of these classes, that
        seems to be the pattern that has previously been used.

     b) I think the Javadoc should say that these methods will always
        throw exceptions.

14. BTreeScan

14.1 fetch()/fetchWithoutQualify()

     a) I do not think it is good that both methods has the same
        Javadoc description.

14.2 positionAtRowLocation ()

     a) See comment 13.1


15. HeapScan

15.1 positionAtRowLocation()

     a) See 13.1 a)

16. DiskHashTable

16.1 Constructor

     a) It seems strange to me that you need special handling in order
        to be able to store RowLocation in the hash tables that is not
        necessary for any other types.  Is the problem the
        getNewNull() is broken for RowLocation?  Maybe that problem
        could be fixed in stead?



> 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,
> TYPE_SCROLL_INSENSITIVE and TYPE_SCROLL_SENSITIVE. The concurrency can
> be one of CONCUR_READ_ONLY and CONCUR_UPDATABLE. The holdability can
> be one of HOLD_CURSORS_OVER_COMMIT and CLOSE_CURSORS_AT_COMMIT.
> JDBC allows the full cross product of these. SQL 2003 prohibits the
> combination {TYPE_SCROLL_INSENSITIVE, CONCUR_UPDATABLE}, but this
> 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:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira

Reply via email to