You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Andreas Korneliussen (JIRA)" <de...@db.apache.org> on 2006/02/28 15:31:41 UTC
[jira] Created: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
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
Assigned to: Andreas Korneliussen
--
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
[jira] Commented: (DERBY-1067) support holdable Scrollable
Updatable Resultsets
Posted by "Dag H. Wanvik (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=comments#action_12369242 ]
Dag H. Wanvik commented on DERBY-1067:
--------------------------------------
I think this seems like a good way to go! By separating out the store
work in this way, the reviewers for SUR (690) would get an easier job
as well. Better with more, smaller patches! So I say, go ahead!
> 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
>
--
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
[jira] Commented: (DERBY-1067) support holdable Scrollable
Updatable Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=comments#action_12370583 ]
Andreas Korneliussen commented on DERBY-1067:
---------------------------------------------
I think currently all the issues have been resolved. So please go ahead and commit this patch.
Thank you all for all the help and feedback you have given me to develop this patch.
--Andreas
> 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, DERBY-1067v3.diff, 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
[jira] Commented: (DERBY-1067) support holdable Scrollable
Updatable Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=comments#action_12369194 ]
Andreas Korneliussen commented on DERBY-1067:
---------------------------------------------
> Suresh Thalamati commented on DERBY-1067:
> -----------------------------------------
>
> This patch seems to do something special for in-place compress making a
> row location invalid, how is in-place compress invalidating a row location
> diffent from say :
>
> 1) a drop table
> 2) compress(SYSCS_COMPRESS_TABLE), that copies rows to a new container.
>
This is different because we are not trying to guarantee that a RowLocation is valid from any container, only from the container it is being used from.
If a container is dropped, a cursor which has an open scan against it should fail.
For holdable cursors, this should fail when trying to reopen the dropped container after a commit. I think this logic already is in place in Derby.
> 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
>
--
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
Re: [jira] Updated: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by Oystein Grovlen - Sun Norway <Oy...@Sun.COM>.
Andreas Korneliussen (JIRA) wrote:
> [ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
>
> Andreas Korneliussen updated DERBY-1067:
> ----------------------------------------
>
> Attachment: DERBY-1067v4.diff
> storeunit_report.txt
>
> Attaching patch to fix some JavaDoc comments. Also including report from storunit suite.
Patch looks good. All my concerns have been addressed. I recommend
that this patch is committed.
--
Øystein
[jira] Updated: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Andreas Korneliussen updated DERBY-1067:
----------------------------------------
Attachment: DERBY-1067v4.diff
storeunit_report.txt
Attaching patch to fix some JavaDoc comments. Also including report from storunit suite.
> 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, DERBY-1067v3.diff, DERBY-1067v4.diff, derbyall_report.txt, storeunit_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
[jira] Closed: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Andreas Korneliussen closed DERBY-1067:
---------------------------------------
> 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
> Fix For: 10.2.0.0
> Attachments: DERBY-1067.diff, DERBY-1067.stat, DERBY-1067v2.diff, DERBY-1067v2.stat, DERBY-1067v3.diff, DERBY-1067v4.diff, derbyall_report.txt, storeunit_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
[jira] Commented: (DERBY-1067) support holdable Scrollable
Updatable Resultsets
Posted by "Øystein Grøvlen (JIRA)" <de...@db.apache.org>.
[ 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
[jira] Closed: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Andreas Korneliussen closed DERBY-1067:
---------------------------------------
> 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, DERBY-1067v3.diff, DERBY-1067v4.diff, derbyall_report.txt, storeunit_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
[jira] Resolved: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "David Van Couvering (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
David Van Couvering resolved DERBY-1067:
----------------------------------------
Resolution: Fixed
Committed revision 386457. Builds clean and runs storeunit with no errors.
> 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, DERBY-1067v3.diff, DERBY-1067v4.diff, derbyall_report.txt, storeunit_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
[jira] Updated: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Andreas Korneliussen updated DERBY-1067:
----------------------------------------
Other Info: [Patch available]
Description:
Environment:
> 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
>
--
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
[jira] Commented: (DERBY-1067) support holdable Scrollable
Updatable Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=comments#action_12370666 ]
Andreas Korneliussen commented on DERBY-1067:
---------------------------------------------
>>>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.
>>>
>>
>>I think I will leave it as is, since I have previously been asked to
>>link this with reusable record ids. However, I do not mind that
>>anyone uses this mechanism for something else, and as part of that
>>issue renames it to something else, like recordIdVersion.
>
>
> You describe the new header field as "The sequence number for reusable
> sequence number." For someone who is just looking at FileContainer
> and not concerned about your specific use of this field, this does not
> make much sense. A meaningful concept for FileContainer is "As long
> as this number does not change, RecordIds will be stable". This
> will also cover those concerned with records being moved, as well as
> those only concerned by reuse of RecordIds.
>
That is a typo. I will fix the comment to say:
"The sequence number for reusable recordIds. As long as this number does not change, recordIds will be stable within the container."
>
>
>>>6. FileContainer header:
>>> a) When looking at the list of fields, it seems like there will
>>> be only 10 bytes of spare space left.
>>
>>One long field (8 bytes) + one integer (4 bytes), should be 12.
>>Do not see how it would only be 10 bytes left.
>
>
> According to the comment spare1 is only 2 bytes.
>
Yes, it seems spare1 is a short. Initially the header said there were 20 bytes left. I made use of 8 bytes, and thought that the header was correct from before and withdrew 8 from 20 and got 12. I will fix the comment.
>
>>>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?)
>>
>>Added.
>
>
> New additions look very good. However, I would be much more comforted
> with a test where compress actually does something. That is,
> tests where records have been deleted. Maybe this is already part of the
> SUR testsuite? We should also have tests that reposition holdable
> cursors on deleted records.
>
This is part of the HoldabilityTest which was submitted as part of DERBY-1070. It tests that a compress actually takes place. It also does some inserts which ensures that recordIds actually are being reused, and it tests that SUR is not able to update the rows. Without this fix, SUR will incorrectly update one of the inserted rows. So, I think the test is sufficent, it verifies that if a compress is being run, the recordIds are invalidated.
> 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, DERBY-1067v3.diff, 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
[jira] Updated: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Mike Matrigali (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Mike Matrigali updated DERBY-1067:
----------------------------------
Description: I am reviewing this patch. (mike matrigali) (was: )
> 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
>
> I am reviewing this patch. (mike matrigali)
--
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
[jira] Updated: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Mike Matrigali (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Mike Matrigali updated DERBY-1067:
----------------------------------
Description: (was: I am reviewing this patch. (mike matrigali)
2 major concerns with this patch:
1) The timestamp is implemented in runtime, but not persistent. The container
can be thrown away as soon as someone is not referencing it, which I believe
can happen in the holdable cursor case. If you want to implement the
timestamp then I think you have to add to the container header
(see FileContainer line 278 for container header description), and
follow code that updates estimated row count for how it is updated and
read. Note that doing this is an UPGRADE issue, and you should think
about soft vs. hard upgrade for this feature.
For more comments about upgrade I need to know your plan. On soft upgrade
will timestamp be bumped or not. I would prefer that it not be changed.
The current assumption for "unused" fields in store is that they are
guaranteed with a specific value (usually 0) before an upgrade. So
on hard upgrade we know the starting value. Also if you change it in
soft upgrade then you have to make sure that all previous of 10.1 don't
have a problem with that field not being 0 - sometimes there are assertions
about the field being 0, don't know for sure in this particular case.
2) I would have expected tests specific to this change associated with the
patch.
some testing areas of concern:
o soft upgrade, make sure 10.1 works correctly on a 10.2 soft upgrade run.
o what happens on timestamp overflow?
minor comments:
general comments:
I would have rather seen the timestamp tied to the reusable rowlocation
concept rather than tied to compress. While true the only thing in the
current code that breaks this is compress, so this may just be my itch.
should timestamp be more "time" related. A single db may reuse a containerid,
but only after a shutdown/reboot cycle. A time based timestamp would mean
the new container timestamp would be different from the old one. Probably
does not matter for held cursors, but what makes sense for the generic new
timestamp feature?
questions:
why do you get the timestamp for the open cursor at close rather than open?
style comments:
don't want to start coding style arg here, and admit not all store code is
perfect. Most the access code is consistent though, and uses the brace on
separate line standard.
GenericScanController.java, reOpenAfterEndTransaction() - coding style does
not match surrounding code (ie. brackets on same lines and if condition
on same line).
GenericScanController.java, closeForEndTransaction() - coding style does
not match code in same routine. I think minimum style in same routine
should be consistent.)
I am reviewing this patch. (mike matrigali)
2 major concerns with this patch:
1) The timestamp is implemented in runtime, but not persistent. The container
can be thrown away as soon as someone is not referencing it, which I believe
can happen in the holdable cursor case. If you want to implement the
timestamp then I think you have to add to the container header
(see FileContainer line 278 for container header description), and
follow code that updates estimated row count for how it is updated and
read. Note that doing this is an UPGRADE issue, and you should think
about soft vs. hard upgrade for this feature.
For more comments about upgrade I need to know your plan. On soft upgrade
will timestamp be bumped or not. I would prefer that it not be changed.
The current assumption for "unused" fields in store is that they are
guaranteed with a specific value (usually 0) before an upgrade. So
on hard upgrade we know the starting value. Also if you change it in
soft upgrade then you have to make sure that all previous of 10.1 don't
have a problem with that field not being 0 - sometimes there are assertions
about the field being 0, don't know for sure in this particular case.
2) I would have expected tests specific to this change associated with the
patch.
some testing areas of concern:
o soft upgrade, make sure 10.1 works correctly on a 10.2 soft upgrade run.
o what happens on timestamp overflow?
minor comments:
general comments:
I would have rather seen the timestamp tied to the reusable rowlocation
concept rather than tied to compress. While true the only thing in the
current code that breaks this is compress, so this may just be my itch.
should timestamp be more "time" related. A single db may reuse a containerid,
but only after a shutdown/reboot cycle. A time based timestamp would mean
the new container timestamp would be different from the old one. Probably
does not matter for held cursors, but what makes sense for the generic new
timestamp feature?
questions:
why do you get the timestamp for the open cursor at close rather than open?
style comments:
don't want to start coding style arg here, and admit not all store code is
perfect. Most the access code is consistent though, and uses the brace on
separate line standard.
GenericScanController.java, reOpenAfterEndTransaction() - coding style does
not match surrounding code (ie. brackets on same lines and if condition
on same line).
GenericScanController.java, closeForEndTransaction() - coding style does
not match code in same routine. I think minimum style in same routine
> 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
>
--
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
[jira] Commented: (DERBY-1067) support holdable Scrollable
Updatable Resultsets
Posted by "?ystein Gr?vlen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=comments#action_12370660 ]
Øystein Grøvlen commented on DERBY-1067:
----------------------------------------
Andreas Korneliussen (JIRA) wrote:
>> Øystein Grøvlen commented on DERBY-1067:
>> ----------------------------------------
>>
>> 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?
>>
>
> No. In order for a RowLocation to be invalidated,the row has to be
> deleted, either as part of a compress (delete+insert) or as part of
> delete+purge. The caller does not need to distinguish between these
> two cases.
I understand that there are two ways for a RowLocation to become
invalid:
1. On the first repositioning after a compress.
2. Repositioning on a deleted&purged row.
Will the holdable cursor be invalidated in both cases, or will one be
able to continue to the next record in the second case?
>
>> 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.
>>
>
> I think I will leave it as is, since I have previously been asked to
> link this with reusable record ids. However, I do not mind that
> anyone uses this mechanism for something else, and as part of that
> issue renames it to something else, like recordIdVersion.
You describe the new header field as "The sequence number for reusable
sequence number." For someone who is just looking at FileContainer
and not concerned about your specific use of this field, this does not
make much sense. A meaningful concept for FileContainer is "As long
as this number does not change, RecordIds will be stable". This
will also cover those concerned with records being moved, as well as
those only concerned by reuse of RecordIds.
>> 6. FileContainer header:
>> a) When looking at the list of fields, it seems like there will
>> be only 10 bytes of spare space left.
>
> One long field (8 bytes) + one integer (4 bytes), should be 12.
> Do not see how it would only be 10 bytes left.
According to the comment spare1 is only 2 bytes.
>> 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?)
> Added.
New additions look very good. However, I would be much more comforted
with a test where compress actually does something. That is,
tests where records have been deleted. Maybe this is already part of the
SUR testsuite? We should also have tests that reposition holdable
cursors on deleted records.
> 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, DERBY-1067v3.diff, 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
[jira] Reopened: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Andreas Korneliussen reopened DERBY-1067:
-----------------------------------------
> 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, DERBY-1067v3.diff, DERBY-1067v4.diff, derbyall_report.txt, storeunit_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
[jira] Commented: (DERBY-1067) support holdable Scrollable
Updatable Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=comments#action_12370334 ]
Andreas Korneliussen commented on DERBY-1067:
---------------------------------------------
I have run this patch along with DERBY-690 and DERBY-1058. With those patches, I have successfully run:
jdbcapi/HoldabilityTest.junit
jdbcapi/ConcurrencyTest.junit
jdbcapi/SURTest.junit
jdbcapi/SURTQueryMixTest.junit
storeunit (suite)
> 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
[jira] Commented: (DERBY-1067) support holdable Scrollable
Updatable Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=comments#action_12368256 ]
Andreas Korneliussen commented on DERBY-1067:
---------------------------------------------
To support holdable SUR, we will invalidate the ResultSet from doing any updates in case of a online compress.
This can be done by:
- A sequence number is associated with each Container
- The sequence number is updated when doing truncate
A holdable cursor will need to reopen the controller after a commit, since the controllers get closed at the end of the transaction (in closeForEndTransaction(..)).
When reopening a controller, one may check that the sequence number has not been changed since it was initially opened. If it has changed, one can conclude that there has been a online compress, and updates cannot be safely executed, and we may reject the reopen.
Any attempt to do update on a non-reopened controller, will fail, and a warning given (cursor operation conflict).
This solution does not have the downside of requiring any changes to the page layout, or RowLocation. It also does not have a cost per row. The downside, is that a online compress will invalidate the cursor from doing any update, even for rows which are unaffected of the truncate.
-
To prevent the solution from having any side-effects on other holdable cursors, we should not generally fail to reopen() a cursor if online compress has run, we should only set a flag in the GenericScanController class.
When the GenericScanController class (or one of its subclasses) calls OpenConglom.reopen(), it can read the timestamp from the container, and based on its own scan_state, and previous timestamp read, it can set a flag (oldRowLocationsInvalid).
The SUR uses a method currently called positionAtRowLocation(..) which it uses to renavigate the controller. This method could check the oldRowLocationsInvalid flag and return false if the old row locations have become invalidated.
So, the setting of the flag, could happen for all holdable cursors, however the call to positionAtRowLocation(..), which is only used by SUR and requires the RowLocation parameter to be a valid row location, is the only call which need to check on that flag, and have logic to fail the operation.
If the positionAtRowLocation(..) call fails, the CurrentOfResultSet's will get a null reference to the RowLocation it is going to update. This will cause a positioned update / delete / updateRow() / deleteRow() to fail, and give a warning (Cursor operation conflict)
> 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
>
--
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
Re: [jira] Commented: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by Mike Matrigali <mi...@sbcglobal.net>.
this sounds great to me.
Andreas Korneliussen (JIRA) wrote:
> [ http://issues.apache.org/jira/browse/DERBY-1067?page=comments#action_12369228 ]
>
> Andreas Korneliussen commented on DERBY-1067:
> ---------------------------------------------
>
> I have been thinking about the testing of this feature. I have already submitted a set of tests in HoldabilityTest.junit, which tests this feature as soon as SUR (DERBY-690) gets reviewed and committed. The test is a pure jdbc test.
>
> However, maybe it would be cleaner if the related store code from DERBY-690 gets moved into this issue ? Then I could write pure store unit tests.
>
> DERBY-690 extends the ScanController with a method called positionAtRowLocation(..). It is in this method one would need to check the rowLocationsInvalidated flag. A store unit test could i.e do the following:
>
> 1. Open a scan
> 2. scan forward, and remeber some RowLocations
> 3. reposition the scan using positionAtRowLocation(..).
>
> The holdability case would be:
> 1. Open a scan
> 2. scan forward, and remeber some RowLocations
> 3. Commit the transaction
> 4. Run compress on the container
> 5. assert that positionAtRowLocation(..) returns false
>
> This would also make the DERBY-690 patch even cleaner.
>
>
>>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
>>
>
>
>
[jira] Commented: (DERBY-1067) support holdable Scrollable
Updatable Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=comments#action_12369228 ]
Andreas Korneliussen commented on DERBY-1067:
---------------------------------------------
I have been thinking about the testing of this feature. I have already submitted a set of tests in HoldabilityTest.junit, which tests this feature as soon as SUR (DERBY-690) gets reviewed and committed. The test is a pure jdbc test.
However, maybe it would be cleaner if the related store code from DERBY-690 gets moved into this issue ? Then I could write pure store unit tests.
DERBY-690 extends the ScanController with a method called positionAtRowLocation(..). It is in this method one would need to check the rowLocationsInvalidated flag. A store unit test could i.e do the following:
1. Open a scan
2. scan forward, and remeber some RowLocations
3. reposition the scan using positionAtRowLocation(..).
The holdability case would be:
1. Open a scan
2. scan forward, and remeber some RowLocations
3. Commit the transaction
4. Run compress on the container
5. assert that positionAtRowLocation(..) returns false
This would also make the DERBY-690 patch even cleaner.
> 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
>
--
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
[jira] Commented: (DERBY-1067) support holdable Scrollable
Updatable Resultsets
Posted by "Suresh Thalamati (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=comments#action_12369115 ]
Suresh Thalamati commented on DERBY-1067:
-----------------------------------------
This patch seems to do something special for in-place compress making a
row location invalid, how is in-place compress invalidating a row location
diffent from say :
1) a drop table
2) compress(SYSCS_COMPRESS_TABLE), that copies rows to a new container.
minor comments:
compressTimestamp is confusing, at least to me :-) I thought you are actually
putting some real time stamp. May be it should be changed to somethig
like "compress version" .
Thanks
-suresh
> 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
>
--
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
[jira] Updated: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Kathey Marsden (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Kathey Marsden updated DERBY-1067:
----------------------------------
Component: JDBC
Network Client
> support holdable Scrollable Updatable Resultsets
> ------------------------------------------------
>
> Key: DERBY-1067
> URL: http://issues.apache.org/jira/browse/DERBY-1067
> Project: Derby
> Type: Sub-task
> Components: JDBC, Network Client
> Reporter: Andreas Korneliussen
> Assignee: Andreas Korneliussen
> Fix For: 10.2.0.0
> Attachments: DERBY-1067.diff, DERBY-1067.stat, DERBY-1067v2.diff, DERBY-1067v2.stat, DERBY-1067v3.diff, DERBY-1067v4.diff, derbyall_report.txt, storeunit_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
[jira] Updated: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Mike Matrigali (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Mike Matrigali updated DERBY-1067:
----------------------------------
Description:
I am reviewing this patch. (mike matrigali)
2 major concerns with this patch:
1) The timestamp is implemented in runtime, but not persistent. The container
can be thrown away as soon as someone is not referencing it, which I believe
can happen in the holdable cursor case. If you want to implement the
timestamp then I think you have to add to the container header
(see FileContainer line 278 for container header description), and
follow code that updates estimated row count for how it is updated and
read. Note that doing this is an UPGRADE issue, and you should think
about soft vs. hard upgrade for this feature.
For more comments about upgrade I need to know your plan. On soft upgrade
will timestamp be bumped or not. I would prefer that it not be changed.
The current assumption for "unused" fields in store is that they are
guaranteed with a specific value (usually 0) before an upgrade. So
on hard upgrade we know the starting value. Also if you change it in
soft upgrade then you have to make sure that all previous of 10.1 don't
have a problem with that field not being 0 - sometimes there are assertions
about the field being 0, don't know for sure in this particular case.
2) I would have expected tests specific to this change associated with the
patch.
some testing areas of concern:
o soft upgrade, make sure 10.1 works correctly on a 10.2 soft upgrade run.
o what happens on timestamp overflow?
minor comments:
general comments:
I would have rather seen the timestamp tied to the reusable rowlocation
concept rather than tied to compress. While true the only thing in the
current code that breaks this is compress, so this may just be my itch.
should timestamp be more "time" related. A single db may reuse a containerid,
but only after a shutdown/reboot cycle. A time based timestamp would mean
the new container timestamp would be different from the old one. Probably
does not matter for held cursors, but what makes sense for the generic new
timestamp feature?
questions:
why do you get the timestamp for the open cursor at close rather than open?
style comments:
don't want to start coding style arg here, and admit not all store code is
perfect. Most the access code is consistent though, and uses the brace on
separate line standard.
GenericScanController.java, reOpenAfterEndTransaction() - coding style does
not match surrounding code (ie. brackets on same lines and if condition
on same line).
GenericScanController.java, closeForEndTransaction() - coding style does
not match code in same routine. I think minimum style in same routine
should be consistent.
was:I am reviewing this patch. (mike matrigali)
> 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
>
> I am reviewing this patch. (mike matrigali)
> 2 major concerns with this patch:
> 1) The timestamp is implemented in runtime, but not persistent. The container
> can be thrown away as soon as someone is not referencing it, which I believe
> can happen in the holdable cursor case. If you want to implement the
> timestamp then I think you have to add to the container header
> (see FileContainer line 278 for container header description), and
> follow code that updates estimated row count for how it is updated and
> read. Note that doing this is an UPGRADE issue, and you should think
> about soft vs. hard upgrade for this feature.
> For more comments about upgrade I need to know your plan. On soft upgrade
> will timestamp be bumped or not. I would prefer that it not be changed.
> The current assumption for "unused" fields in store is that they are
> guaranteed with a specific value (usually 0) before an upgrade. So
> on hard upgrade we know the starting value. Also if you change it in
> soft upgrade then you have to make sure that all previous of 10.1 don't
> have a problem with that field not being 0 - sometimes there are assertions
> about the field being 0, don't know for sure in this particular case.
> 2) I would have expected tests specific to this change associated with the
> patch.
> some testing areas of concern:
> o soft upgrade, make sure 10.1 works correctly on a 10.2 soft upgrade run.
> o what happens on timestamp overflow?
> minor comments:
> general comments:
> I would have rather seen the timestamp tied to the reusable rowlocation
> concept rather than tied to compress. While true the only thing in the
> current code that breaks this is compress, so this may just be my itch.
> should timestamp be more "time" related. A single db may reuse a containerid,
> but only after a shutdown/reboot cycle. A time based timestamp would mean
> the new container timestamp would be different from the old one. Probably
> does not matter for held cursors, but what makes sense for the generic new
> timestamp feature?
> questions:
> why do you get the timestamp for the open cursor at close rather than open?
> style comments:
> don't want to start coding style arg here, and admit not all store code is
> perfect. Most the access code is consistent though, and uses the brace on
> separate line standard.
> GenericScanController.java, reOpenAfterEndTransaction() - coding style does
> not match surrounding code (ie. brackets on same lines and if condition
> on same line).
> GenericScanController.java, closeForEndTransaction() - coding style does
> not match code in same routine. I think minimum style in same routine
> should be consistent.
--
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
Re: [jira] Updated: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by Mike Matrigali <mi...@sbcglobal.net>.
I will review this patch, but probably won't get to it until monday. If
anyone else is reviewing, let me know so I can know when/if commit
should happen.
Andreas Korneliussen (JIRA) wrote:
> [ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
>
> Andreas Korneliussen updated DERBY-1067:
> ----------------------------------------
>
> Attachment: DERBY-1067v2.diff
> DERBY-1067v2.stat
>
> Attached is an updated patch. This patch includes changes from DERBY-690 to do
> positioning at row location.
>
> The sequence number has been associated with reusable record id, and is now called "reusableRecordIdSequenceNumber"
> instead of "compressTimeStamp".
>
> The sequence number is incremented from BaseContainer.compressContainer(..). I noticed that a purge operation
> may cause pages with no records to be deallocated, so I have considered also incrementing the number when
> removing pages. However, some testing indicated to me that when a page is reused after being deallocated by a
> purge, the recordId is not reused within the page. It seems only truncate will cause the recordId to be reused.
> (truncate uses BaseContainer.compressContainer(..)).
Yes purge of page is not a problem. Note these purges happen all the
time in a db with deletes taking place, they are not specific to truncate.
>
> Another change is that when incrementing the reusable record id sequenence number, the isDirty flag may also be set, to
> ensure the container header gets written do disk.
>
> Testing:
>
> T_AcessFactory has been extended with a test which checks that a compress
> will update the sequence number, so that the positionAtRowLocation(..) returns false.
>
> The phaseTester has been extended to test that the version sequence number does not
> harm a 10.1 Database.
>
> I have currently only run these tests with this patch.
> I will start derbyall now, and also test this patch along with DERBY-690 and DERBY-1058.
>
>
>
>>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
>>
>
>
>
[jira] Updated: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Andreas Korneliussen updated DERBY-1067:
----------------------------------------
Attachment: DERBY-1067v2.diff
DERBY-1067v2.stat
Attached is an updated patch. This patch includes changes from DERBY-690 to do
positioning at row location.
The sequence number has been associated with reusable record id, and is now called "reusableRecordIdSequenceNumber"
instead of "compressTimeStamp".
The sequence number is incremented from BaseContainer.compressContainer(..). I noticed that a purge operation
may cause pages with no records to be deallocated, so I have considered also incrementing the number when
removing pages. However, some testing indicated to me that when a page is reused after being deallocated by a
purge, the recordId is not reused within the page. It seems only truncate will cause the recordId to be reused.
(truncate uses BaseContainer.compressContainer(..)).
Another change is that when incrementing the reusable record id sequenence number, the isDirty flag may also be set, to
ensure the container header gets written do disk.
Testing:
T_AcessFactory has been extended with a test which checks that a compress
will update the sequence number, so that the positionAtRowLocation(..) returns false.
The phaseTester has been extended to test that the version sequence number does not
harm a 10.1 Database.
I have currently only run these tests with this patch.
I will start derbyall now, and also test this patch along with DERBY-690 and DERBY-1058.
> 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
>
--
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
[jira] Commented: (DERBY-1067) support holdable Scrollable
Updatable Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=comments#action_12370335 ]
Andreas Korneliussen commented on DERBY-1067:
---------------------------------------------
Note: I had to do some changes to the DERBY-690v2 patch, since some of its code was moved into this patch.
> 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
[jira] Updated: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Andreas Korneliussen updated DERBY-1067:
----------------------------------------
Attachment: DERBY-1067.diff
DERBY-1067.stat
Attached is a diff (DERBY-1067.diff) which implements the necessary logic in store. This diff is independent from DERBY-690.
The GenericScanController get a flag, rowLocationInvalidated, and a timestamp. The timestamp is initialized when the controller is closed as part of a commit, and later compared when the controller is reopened in the new transaction. The flag is set on reopen.
I am now in the process of running derbyall with this patch.
> 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
>
--
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
[jira] Updated: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Andreas Korneliussen updated DERBY-1067:
----------------------------------------
Attachment: derbyall_report.txt
Attaching report from derbyall.
One test fails due to DERBY-734. Reran the test two times, and it succeeded one of the times.
The other test seems to fail from a clean sandbox as well, and I have previously analyzed it to be some policy problem when starting the network server.
> 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
[jira] Resolved: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Andreas Korneliussen resolved DERBY-1067:
-----------------------------------------
Fix Version: 10.2.0.0
Resolution: Fixed
Derby Info: [Patch Available]
New feature
> 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
> Fix For: 10.2.0.0
> Attachments: DERBY-1067.diff, DERBY-1067.stat, DERBY-1067v2.diff, DERBY-1067v2.stat, DERBY-1067v3.diff, DERBY-1067v4.diff, derbyall_report.txt, storeunit_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
[jira] Updated: (DERBY-1067) support holdable Scrollable Updatable
Resultsets
Posted by "Andreas Korneliussen (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1067?page=all ]
Andreas Korneliussen updated DERBY-1067:
----------------------------------------
Attachment: DERBY-1067v3.diff
Thanks for the review.
Attached is a new patch which improves the unit test as requested. In addition, the unit test now also uses fetch() to verify the contents of the row. In addition, the patch fixed one comment (due to question/comment 4).
Below are answers to your questions (inline):
>
> Ø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().
>
Yes. This is the idea. next() positions the scan to the next valid row. If I did anything to invalidate next(), it would have side-effects on forward only cursors.
> 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?
>
No. In order for a RowLocation to be invalidated,the row has to be deleted, either as part of a compress (delete+insert) or as part of delete+purge.
The caller does not 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.
>
I think I will leave it as is, since I have previously been asked to link this with reusable record ids. However, I do not mind that anyone uses this mechanism for something else, and as part of that issue renames it to something else, like recordIdVersion.
> 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.
>
Yes. I fixed the comment.
> 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.
One long field (8 bytes) + one integer (4 bytes), should be 12.
Do not see how it would only be 10 bytes 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?)
Added.
> b) Comments says that an index is created on the conglomerate.
> I do not see any code for that.
Fixed comment.
> c) Why does one fetch the row locations when they are not used
> for anything? Why not use plain insert() instead of
> insertAndFetchLocation()?
>
Because this was cut'npasted from heldCursor. I have changed it to use insert() instead.
> 8. phaseTester: Any particular reason a prepared statement is used
> for the compresssion?
>
No, it can be used with plain statements also.
> 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, DERBY-1067v3.diff, 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