[
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
