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-1067) support holdable Scrollable Updatable Resultsets
Date Tue, 14 Mar 2006 16:51:41 GMT
    [ http://issues.apache.org/jira/browse/DERBY-1067?page=comments#action_12370368 ] 

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

Patch looks good, but I have a few questions/comments:

1. Is the idea that as long as you go forward and do not reposition,
   one should be allowed to proceed after a compress?  I ask because I
   have not quite understood why invalid row locations only seem to
   have effect on positionOnRowLocation() and not next().

2. It seems to me that positionOnRowLocation() can return false in two
   cases: 
      1) The row location is no longer valid
      2) The record at this location has been deleted
   Does not a caller need to distinguish between these two cases?

3. I suggest to use something even more generic than
   reusableRecordIdSequenceNumber.  How about recordIdVersion or
   something like that?  I agree that what we worry about here is
   reuse of record ids, but I think this mechanism could be used for
   other purposes too.

4. reopenAfterEndTransaction has a comment that says "Only reopen
   holdable conglomerates".  I guess it is not the conglomerate that
   is holdable, but the way it was opened.

5. The following is not an objection to this patch, but the existing
   code: HeapCompressScan.fetchRowsForCompress() has copied a lot of
   code from GenericScanController.fetchRows().  This requires this
   patch to update both methods.  Is it not possible to organize this
   in a way that avoids this code duplication?  Mike, can comment on
   this?

6. FileContainer header:  
      a) When looking at the list of fields, it seems like there will
         be only 10 bytes of spare space left.
      b) We will now have a spare1 and a spare3 field, but no spare2
         field.  I guess that may confuse some people, but renaming
         spare3 to spare2 may also create confusion.  I am not sure
         what is best.

7. Unit test:
      a) I think it would also be good with a test that does next()
         after a compress and verifies that it is positioned at the
         correct row.  (Or maybe this is already part of the SUR
         testsuite?)
      b) Comments says that an index is created on the conglomerate.
         I do not see any code for that.
      c) Why does one fetch the row locations when they are not used
         for anything?  Why not use plain insert() instead of
         insertAndFetchLocation()?

8. phaseTester:  Any particular reason a prepared statement is used
   for the compresssion?


> support holdable Scrollable Updatable Resultsets
> ------------------------------------------------
>
>          Key: DERBY-1067
>          URL: http://issues.apache.org/jira/browse/DERBY-1067
>      Project: Derby
>         Type: Sub-task
>     Reporter: Andreas Korneliussen
>     Assignee: Andreas Korneliussen
>  Attachments: DERBY-1067.diff, DERBY-1067.stat, DERBY-1067v2.diff, DERBY-1067v2.stat,
derbyall_report.txt
>


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