[
https://issues.apache.org/jira/browse/DERBY-2462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491102
]
A B commented on DERBY-2462:
----------------------------
Thank you for considering my feedback, Dag. The v3 patch is indeed cleaner now.
Two follow-up questions that occur to me:
1) The new "isPositioned()" method checks to see if the current scan_state is
"SCAN_INPROGRESS". This works well enough for the case we're trying to
address--i.e. the scan was closed due to a commit and is therefore no longer
"in progress".
My question is: are there any scenarios in which we could get to the new code
when the scan was closed for some reason *other* than a commit? If this is
possible, will attempting to re-open the scan in such a scenario cause problems?
I don't know enough about the lifetime of a scan to give any examples of when
or if this could happen, but given the generality of the isPositioned() method
I thought I'd bring it up.
I did notice that one of the possible scan states is (pulled from BTreeScan):
* SCAN_HOLD_INPROGRESS -
* The scan has been opened and held open across a commit,
* at the last commit the state was in SCAN_INPROGRESS.
* The transaction which opened the scan has committed,
* but the scan was opened with the "hold" option true.
* At commit the locks were released and the "current"
* position is remembered. In this state only two calls
* are valid, either next() or close(). When next() is
* called the scan is reopened, the underlying container
* is opened thus associating all new locks with the current
* transaction, and the scan continues at the "next" row.
To see what would happen I changed "isPositioned()" method to:
public boolean isPositioned() throws StandardException
{
return scan_state == SCAN_HOLD_INPROGRESS;
}
and then dropped the negation from the check in DiskHashtable:
if (keepAfterCommit && scan.isPositioned()) {
// automatically reopens scan:
if (!scan.positionAtRowLocation(rowloc)) {
// Will not happen unless compress of this table
// has invalidated the row location. Possible?
throw StandardException.
newException(SQLState.NO_CURRENT_ROW);
}
}
I then ran lang/SpillHash.java and the test still passed. The good thing
about this approach is that we will only execute the "reopen" logic if we know
for a fact that the "scan has been opened and held open across a commit"--which
is exactly what we want. Also, if we rename "isPositioned()" to something more
appropriate, the "if" statement in DiskHashtable becomes more intuitive:
if (scan.heldAcrossCommit()) {
// automatically reopens scan:
if (!scan.positionAtRowLocation(rowloc)) {
Note that you wouldn't need to include "keepAfterCommit" anymore because the
scan state can only be "HOLD_INPROGRESS" if the scan was opened with "hold" set
to true, which (I think?) can only happen if "keepAfterCommit" is true.
2) I noticed that the lang/SpillHash.java test checks to make sure that the
scan is still available after a commit if holdOverCommit is true. This is good
because that's the whole point of DERBY-2462. I was wondering, though, if it
would be worth it to check that the correct *error* is thrown if a call to
"next()" is made after a commit when holdOverCommit is *false*. Or is that
already tested somewhere else?
Thanks for your patience with my ramblings...
> 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, DERBY-2462-3.diff, DERBY-2462-3.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.
