db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "A B (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-2462) org.apache.derby.impl.store.access.BackingStoreHashTableFromScan does not honor ResultSet holdability
Date Fri, 20 Apr 2007 22:05:16 GMT

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


Mime
View raw message