Result sets of type TYPE_SCROLL_INSENSITIVE should not implicitly close due to positioning in autocommit mode

• Key: DERBY-1295
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Minor
• Assignee: Fernanda Pizzorno
• Reporter: Dag H. Wanvik
• Component/s: JDBC
• Affects Version/s: 10.2.1.6
• Fix Version/s: 10.2.1.6

Description

The new JDBC 4 specification allows implementations to automatically
close result sets of type FORWARD_ONLY when ResultSet#next returns
false:

(quote from JDBC preliminary spec):

> 16.2.5 Closing a ResultSet Object
> :
> NOTE: Some JDBC driver implementations may also implicitly close the
> ResultSet when the ResultSet type is TYPE_FORWARD_ONLY and the next
> method of ResultSet returns false.

This implies that other result set type are not free to do this.

Currently, Derby will also implicitly close result sets of type
TYPE_SCROLL_INSENSITIVE, if autocommit is enabled.

Quote from Derby Developer's Guide, subsection "Using autocommit":
 
> Using auto-commit
>
> A new connection to a Derby database is in auto-commit mode by
> default, as specified by the JDBC standard. Auto-commit mode means
> that when a statement is completed, the method commit is called on
> that statement automatically. Auto-commit in effect makes every SQL
> statement a transaction. The commit occurs when the statement
> completes or the next statement is executed, whichever comes
> first. In the case of a statement returning a ResultSet , the
> statement completes when the last row of the ResultSet has been
****************************************
> retrieved or the ResultSet has been closed explicitly.

This seems to indicate that result set always closes when the last row
has been seen, however, it seems the implementation only does this
when autocommit is enabled. I will attach a repro.

Anyway, this should be corrected for JDBC4 compliancy. Scrollable
result sets should never close implicitly due to positioning,
autocommit or not.

Comments

[Dag H. Wanvik]

Repro for embedded; it seems not to happen for client,
maybe because the next is handled locally when already on last row.

[Dag H. Wanvik]

This behavior is also mentioned in the reference manual (src/ref/rrefjdbc77156.dita):

  "Note: When working with scrolling insensitive ResultSets when
  auto-commit mode is turned on, the only positioning method that
  can close the ResultSet automatically is the next() method."

[Fernanda Pizzorno]

The attached file derby-1295.diff adds a check in EmbedResultSet.java so that only forward only result sets will close due to positioning, and a new test (jdbcapi/ScrollResultSetTest.junit). I have successfully run derbyall with this patch. Can someone please review it?

Thank you in advance!

[Dag H. Wanvik]

Changing this might possibly impact an existing application if it
relies on auto-close of the result as described.

[Kathey Marsden]

Thanks for marking this Existing Application Impact & Release Note Needed and thinking about existing users.

I agree it would be good to document. the change. I think it is not likely to show up as a regression in a user application as I don't think there are too many cases where an app would have functionality depending on auto-close of the result. Ultimately I think they can still rely on garbage collection to perform that function.

[Andreas Korneliussen]

Thanks for contributing this patch.
I have added a subtask to fix the documentation issue.

Here are some minor review comments:

1. I think the patch could improve on the existing nesting of if-else statements , instead it introduces one more level:

if (!onRow && (position == NEXT)) {
if (forMetaData && (lcc.getActivationCount() > 1)) {
} else if (owningStmt != null)
if (owningStmt.getResultSetType() == TYPE_FORWARD_ONLY) {
owningStmt.resultSetClosing(this);
}
}

One could use one if statment:
if (position == NEXT && !onRow &&
        !(forMetaData && (lcc.getActivationCount() > 1)) &&
        owningStmt!=null &&
        owningStmt.getResultSetType() == TYPE_FORWARD_ONLY) {
        
        owningStmt.resultSetClosing(this);
}

or, if you do not want to touch too much of the exisiting structure:

if (!onRow && (position == NEXT)) {

    if (forMetaData && (lcc.getActivationCount() > 1)) {
    } else if (owningStmt != null && owningStmt.getResultSetType() == TYPE_FORWARD_ONLY) {
        owningStmt.resultSetClosing(this);
    }

}

2.
The constructor of ScrollResultSetTest has a javadoc comment which should be fixed:
+ /** Creates a new instance of SURBaseTest */
+ public ScrollResultSetTest(String name) {
+ super(name);
+ }

I am attaching a new patch (derby-1295v2.diff) addressing the review comments. Can someone please review it?

Thank you in advance.

[Andreas Korneliussen]

Patch looks good - I will run some test and commit if successful.

[Andreas Korneliussen]

Some preliminary testing results:
* When running the new test, without the patch, the test failed as expected in embedded
* I also found that the test did not fail when running in DerbyNetClient (also expected)
* When running in DerbyNet, I found it failing with:

1) testNextOnLastRowForwardOnly(org.apache.derbyTesting.functionTests.tests.jdbcapi.ScrollResultSetTest)junit.framework.ComparisonFailure: expected:<null> but was:<XCL16>
(assertEquals has been called with parameters in wrong order: expected is XCL16, not <null>)

Instead of disabling the test in DerbyNet, the test code can use the method usingDerbyNet() to disable checking the SQL state.
In addition, use the method assertSQLState(..) instead of assertEquals(..), and move the sql state string constant into the recently introduced file SQLStateConstants.

I have made these changes in my sandbox, and my intention is now to upload the updated patch, run derbyall and commit.

[Andreas Korneliussen]

Attaching patch derby-1295 with minor update of testcode as commented above.

[Andreas Korneliussen]

Committed revision 412831.

[Fernanda Pizzorno]

Release note for this issue:


PROBLEM
SCROLL_INSENSITIVE result sets in auto commit mode close implicitly when calling the ResultSet.next() method while positioned on the last row.

SYMPTOMS
Calling the ResultSet.next() method when positioned on the last row of a result set of type SCROLL_INSENSITIVE in auto commit mode causes the result set to be closed.

CAUSE
The JDBC specification allows a JDBC driver to implicitly close a ResultSet when the ResultSet type is TYPE_FORWARD_ONLY and the next method of ResultSet returns false. Derby also implicitly closes result sets of type SCROLL_INSENSITIVE when the ResultSet.next() method returns false in auto commit mode.

SOLUTION
The behavior of SCROLL_INSENSITIVE result sets in auto commit mode has been changed to comply with the JDBC4 specification. SCROLL_INSENSITIVE result sets are not implicitly closed when calling the ResultSet.next() method in auto commit mode while positioned on the last row.

WORKAROUND