[
https://issues.apache.org/jira/browse/DERBY-2462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12490462
]
A B commented on DERBY-2462:
----------------------------
I am not familiar with this area of code so I cannot make any definitive
comments on whether or not the changes are correct nor on if there may be a
better way. I did look at the patch, though, and had the following two
questions. These are both minor and somewhat theoretical, so I don't think
either should block commit of the patch. Just my proverbial two cents...
1) The interaction of the various ScanController interfaces and implementations
has me completely confused (which has nothing to do with your changes) but I
did notice one interface called ScanManager, which extends ScanController, that
defines a "closeForEndTransaction(boolean)" method:
http://db.apache.org/derby/javadoc/engine/org/apache/derby/iapi/store/access/conglomerate/ScanManager.html
I also noticed that most of the Scan implementations apparently implement the
ScanManager class. At least, that's what I gather from the above javadoc page;
I tried to verify that by looking at the actual code but got lost in no time.
In any event, I was wondering if that method could somehow be used to indicate
that a ScanController has been closed as the result of a commit. Then instead
of catching an exception in DiskHashtable, you could try something like:
try
{
// DERBY-2462: if holdable and scan got closed due
// to commit, we need to reopen where we left off
if (scan.closedByCommit() && keepAfterCommit)
{
scan = openRowConglomerate(tc, rowConglomerateId);
scan.positionAtRowLocation(rowloc);
}
scan.fetch(row);
where "scan.closedByCommit()" would return true if the scan was closed by a
ScanManager.closeForEndTransaction() call.
As I said, that's just a theoretical. I don't know if it's actually possible
to get something like that working, but since I noticed the method I thought
I'd ask.
If that type of check is not possible then it seems like we should at least be
able to add an "isClosed()" or "isPositioned()" method to the ScanController
that could be used instead of catching the exception. Is there something in
particular that would make such an aproach infeasible?
2) The patch includes the following diff in DiskHashtable:
+ // DERBY-2462: if holdable and scan got closed due
+ // to commit, we need to reopen where we left off
+ if (keepAfterCommit && (SQLState.AM_SCAN_NOT_POSITIONED.
+ substring(0,5).
+ equals(e.getSQLState()))) {
+ scan = openRowConglomerate(tc, rowConglomerateId);
+ scan.positionAtRowLocation(rowloc);
It looks like we first make a call to reopen the scan, then we call
"positionAtRowLocation()". But the javadoc for the latter method (see
ScanController.java) seems to indicate that the reopen happens automatically:
/**
* Positions the scan at row location and locks the row.
* If the scan is not opened, it will be reopened if this is a holdable
* scan and there has not been any operations which causes RowLocations
* to be invalidated.
Is this javadoc correct? If so, then is it necessary to explicitly call
"openRowConglomerate()" in the above diff, or could we get by without it?
Also, the javadoc mentions that "positionAtRowLocation()" locks the row. The
javadoc for ScanController.fetch() doesn't mention anything about locking the
row, but I'm assuming it happens anyway...is that correct? (excuse my ignorance
here). Can you confirm that row locking occurs as expected with these changes?
I have no reason to believe otherwise, just thought I'd ask.
It's worth repeating here that I am not familiar with this area of code and
thus this feedback could be of little-to-no-value, so please keep that in mind.
If any committer out there (including you) wishes to commit these changes as
they are, I would not complain.
On a more concrete level, I verified that the new lang/SpillHash.java test runs
cleanly with your changes and fails with error XSCH6 without them, as expected.
So thanks for fixing that test up.
> org.apache.derby.impl.store.access.BackingStoreHashTableFromScan does not
> honor ResultSet holdability
> -----------------------------------------------------------------------------------------------------
>
> Key: DERBY-2462
> URL: https://issues.apache.org/jira/browse/DERBY-2462
> Project: Derby
> Issue Type: Bug
> Components: Store
> Affects Versions: 10.1.1.0, 10.1.2.1, 10.1.3.1, 10.2.1.6, 10.2.2.0
> Environment: Test under Windows Vista, Java 1.4.2_13
> Reporter: Jeff Clary
> Assigned To: Dag H. Wanvik
> Attachments: DERBY-2462-1.diff, DERBY-2462-1.stat, DERBY-2462-2.diff,
> DERBY-2462-2.stat, DerbyHoldabilityTest.java
>
>
> After an unrelated statement on the same connection commits, and after some
> number of successful calls to ResultSet.next(), a subsequent call to
> ResultSet.next() throws an SQLException with a message like: The heap
> container with container id Container(-1, 1173965368428) is closed. This
> seems to be related to the hard-coded passing of false to the super in the
> constructor of
> org.apache.derby.impl.store.access.BackingStoreHashTableFromScan.
> Steps to reproduce:
> 1. Execute a statement on a connection that returns a result set.
> 2. Execute a second statement on the same connection that modifies the
> database and commits.
> 3. Call next() on the first result set until the exception is thrown.
> Note that the number of rows that can be successfully retrieved from the
> result set seems to be related to the amount of data per row. Increasing the
> number of columns in the result set or the length of the columns causes the
> exception to be taken sooner.
> The attached test program demonstrates the issue.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
