I've attached an "alpha" patch for the DERBY-213 for anyone who is
interested in the DERBY-213 issue. It should be noted that the patch is
contingent upon our current interpretation of the JDBC
specifications/documentation. This patch is meant to address both the
early close problem recently discussed in Derby-user and incorrect
auto-commit behavior for multiple ResultSets. It is *not* meant to
address the issue of DatabaseMetaData ResultSets invoking an
auto-commit. I may file this as a seperate issue in the future. I call
this patch an alpha patch because there is one known bug associated with
it and potentially other unspotted ones. The bug in question occurs in
the lang/updatableResultSet.java test
specifically a call to:
System.out.println("Have this call to rs.rowDeleted() just to make sure
the method does always return false? " + rs.rowDeleted());
rs.deleteRow(); //This line, approximately line 896 of the
test file.
System.out.println("Have this call to rs.rowDeleted() just to make
sure the method does always return false? " + rs.rowDeleted());
I will continue my attempts to establish why this is a bug but as always
input is appreciated.
Philip
Index: am/Connection.java
===================================================================
--- am/Connection.java (revision 191177)
+++ am/Connection.java (working copy)
@@ -515,10 +515,12 @@
}
// precondition: autoCommit_ is true
- public void flowAutoCommit() throws SqlException {
+ public boolean flowAutoCommit() throws SqlException {
if (willAutoCommitGenerateFlow()) {
flowCommit();
+ return true;
}
+ return false;
}
public boolean willAutoCommitGenerateFlow() throws
org.apache.derby.client.am.SqlException {
@@ -1638,4 +1640,10 @@
inUnitOfWork_ = inUnitOfWork;
}
+ public void markAutoCommitted() {
+ java.util.ListIterator iter = openStatements_.listIterator();
+ while(iter.hasNext()) {
+ ((Statement)iter.next()).markAutoCommitted();
+ }
+ }
}
Index: am/Statement.java
===================================================================
--- am/Statement.java (revision 191177)
+++ am/Statement.java (working copy)
@@ -459,6 +459,11 @@
} else {
flowCloseOutsideUOW();
}
+
+ //The connection will not auto commit on close in the case of
+ //multiple open resultsets. This call is here to insure proper
+ //autocommit.
+ connection_.flowAutoCommit();
} finally {
markClosed();
connection_.openStatements_.remove(this);
@@ -1240,33 +1245,29 @@
//
// This is fixed by passing a flag to our statement close processing that
prevents
// driving additional auto-commits after each statement close.
- // Connectino close drives its own final auto-commit.
+ // Connection close drives its own final auto-commit.
//
boolean writeCloseResultSets(int number, boolean allowAutoCommits) throws
SqlException {
- boolean requiresAutocommit = false;
+ ResultSet crs = resultSet_;
if (resultSetList_ != null) {
+ //TODO Potential for ArrayIndexOutOfBoundsException. Fix?
for (int i = 0; i < number; i++) {
if (resultSetList_[i] != null) {
if (resultSetList_[i].openOnServer_) {
resultSetList_[i].writeClose();
}
- if (!resultSetList_[i].autoCommitted_ && allowAutoCommits)
{
- requiresAutocommit = true;
- }
}
}
+ crs = resultSetList_[number - 1];
} else if (generatedKeysResultSet_ != null &&
generatedKeysResultSet_.openOnServer_) {
generatedKeysResultSet_.writeClose();
} else if (resultSet_ != null) {
if (resultSet_.openOnServer_) {
resultSet_.writeClose();
}
- if (!resultSet_.autoCommitted_ && allowAutoCommits) {
- requiresAutocommit = true;
- }
}
- if (connection_.autoCommit_ && requiresAutocommit &&
isAutoCommittableStatement_) {
- connection_.writeAutoCommit();
+ if (connection_.autoCommit_ && isAutoCommitRequired(crs) &&
isAutoCommittableStatement_) {
+ resultSetCommitting(true);
if (connection_.isXAConnection_) {
return (connection_.xaState_ ==
Connection.XA_T0_NOT_ASSOCIATED) ;
} else {
@@ -1283,8 +1284,18 @@
}
void readCloseResultSets(int number, boolean allowAutoCommits) throws
SqlException {
+ /* The call to isAutoCommitRequired must be done before
+ * the readClose() is done. The read close can mark ResultSets
+ * closed. If these Resultsets were open in the writeCloseResultSets
+ * method then we would have a read without the associated write
+ * which will cause a hang.
+ */
boolean requiredAutocommit = false;
+ ResultSet crs = resultSet_;
if (resultSetList_ != null) {
+ //TODO Potential for ArrayIndexOutOfBoundsException. Fix?
+ requiredAutocommit = isAutoCommitRequired(
+ resultSetList_[number - 1]);
for (int i = 0; i < number; i++) {
if (resultSetList_[i] != null) {
if (resultSetList_[i].openOnServer_) {
@@ -1292,9 +1303,6 @@
} else {
resultSetList_[i].markClosed();
}
- if (!resultSetList_[i].autoCommitted_ && allowAutoCommits)
{
- requiredAutocommit = true;
- }
}
}
} else if (generatedKeysResultSet_ != null) {
@@ -1304,14 +1312,12 @@
generatedKeysResultSet_.markClosed();
}
} else if (resultSet_ != null) {
+ requiredAutocommit = isAutoCommitRequired(resultSet_);
if (resultSet_.openOnServer_) {
resultSet_.readClose();
} else {
resultSet_.markClosed();
}
- if (!resultSet_.autoCommitted_ && allowAutoCommits) {
- requiredAutocommit = true;
- }
}
// we only commit when auto commit is turned on and at least one
result set needed closing on server.
if (connection_.autoCommit_ && requiredAutocommit &&
isAutoCommittableStatement_) {
@@ -1326,9 +1332,9 @@
// last result set closed in a set of multiple result sets of the
owning statement,
// if so, we need to flow the auto-commit (but only then).
// currently, the code to do this is only in the closeX() path, which
isn't called here
- writeCloseResultSets(numberOfResultSetsToClose, false);
+ writeCloseResultSets(numberOfResultSetsToClose, true);
agent_.flow(this);
- readCloseResultSets(numberOfResultSetsToClose, false); // true means
permit auto-commits
+ readCloseResultSets(numberOfResultSetsToClose, true); // true means
permit auto-commits
agent_.endReadChain();
}
@@ -1339,9 +1345,9 @@
// last result set closed in a set of multiple result sets of the
owning statement,
// if so, we need to flow the auto-commit (but only then).
// currently, the code to do this is only in the closeX() path, which
isn't called here
- writeCloseResultSets(numberOfResultSetsToClose, false);
+ writeCloseResultSets(numberOfResultSetsToClose, true);
agent_.flowOutsideUOW();
- readCloseResultSets(numberOfResultSetsToClose, false); // true means
permit auto-commits
+ readCloseResultSets(numberOfResultSetsToClose, true); // true means
permit auto-commits
agent_.endReadChain();
}
@@ -2107,5 +2113,60 @@
connection_.clientCursorNameCache_.remove(cursorName_);
cursorName_ = null;
}
+ }
+
+ /**
+ * Method that checks to see if any other ResultSets are open. If not
+ * proceeds with the autocommit.
+ *
+ * @param closingRS The ResultSet to be closed
+ * @param writeChain A Boolean indicating whether this method
+ * is part of a chain of write from client to Server
+ * @throws SqlException
+ */
+ public void resultSetCommitting(boolean writeChain) throws SqlException {
+ if (writeChain) {
+ connection_.writeAutoCommit();
+ } else {
+ if (connection_.flowAutoCommit())
+ connection_.markAutoCommitted();
+ }
+ }
+
+ public void markAutoCommitted() {
+ if (resultSetList_ != null) {
+ for (int i = 0; i < resultSetList_.length; i++) {
+ resultSetList_[i].markAutoCommitted();
+ }
+ }
+ if (resultSet_ != null) resultSet_.markAutoCommitted();
}
+
+ /**
+ * Method to check to see if there needs to be a commit.
+ * @param committingRS
+ * @return
+ */
+ public boolean isAutoCommitRequired(ResultSet committingRS) {
+ // If the Connection is not in auto commit then this statement
completion
+ // cannot cause a commit.
+ if (!connection_.autoCommit_ || committingRS == null ||
committingRS.autoCommitted_)
+ return false;
+
+ if (resultSetList_ != null) {
+ for (int i = 0; i < resultSetList_.length; i++) {
+ ResultSet currentRS = resultSetList_[i];
+ if (currentRS == null)
+ continue;
+ if (!currentRS.openOnClient_)
+ continue;
+ if (currentRS == committingRS)
+ continue;
+
+ // at least one still open so no commit now.
+ return false;
+ }
+ }
+ return true;
+ }
}
Index: am/ResultSet.java
===================================================================
--- am/ResultSet.java (revision 191177)
+++ am/ResultSet.java (working copy)
@@ -283,14 +283,13 @@
// we will close the result set, the autocommit logic is in the
closeX() method
// if (!isValidCursorPosition_ && // We've gone past the end (+100)
// cursor_ != null) {
- if ((!isValidCursorPosition_ && cursor_ != null) ||
- (statement_.maxRows_ > 0 && cursor_.rowsRead_ >
statement_.maxRows_)) {
+ if (statement_.maxRows_ > 0 && cursor_.rowsRead_ >
statement_.maxRows_) {
isValidCursorPosition_ = false;
-
+
// if not on a valid row and the query is closed at the server.
// check for an error which may have caused the cursor to
terminate.
// if there were no more rows because of an error, then this
method
- // should throw an SqlException rather than just returning
false.
+ // should throw an SqlException rather than
just returning false.
// note: closeX is still called and this will cause the
// result set to be closed on the client. any additional calls
to
// next() will fail checkForClosedResultSet(), the query
terminating exception is
@@ -306,14 +305,12 @@
sqlException = new SqlException(agent_.logWriter_,
queryTerminatingSqlca_);
}
}
- try {
- closeX(); // the auto commit logic is in closeX()
- } catch (SqlException sqle) {
- sqlException = Utils.accumulateSQLException(sqle,
sqlException);
- }
if (sqlException != null) {
throw sqlException;
}
+
+ if (statement_.isAutoCommitRequired(this))
+ statement_.resultSetCommitting(false);
}
}
@@ -355,9 +352,7 @@
// for forward-only cursors, check if rowsRead_ > maxRows_.
// for scrollable cursors, check if absolute row number > maxRows_.
// maxRows_ will be ignored by sensitive dynamic cursors since we
don't know the rowCount
- if (!openOnClient_) {
- isValidCursorPosition_ = false;
- } else if (sensitivity_ != sensitivity_sensitive_dynamic__ &&
statement_.maxRows_ > 0 &&
+ if (sensitivity_ != sensitivity_sensitive_dynamic__ &&
statement_.maxRows_ > 0 &&
(firstRowInRowset_ + currentRowInRowset_ >
statement_.maxRows_)) {
isValidCursorPosition_ = false;
}
@@ -383,16 +378,17 @@
}
try {
if (openOnServer_) {
- flowCloseAndAutoCommitIfNotAutoCommitted();
+ flowCloseAndAutoCommit();
} else {
- flowAutoCommitIfNotAutoCommitted(); // in case of early close
+ if (statement_.isAutoCommitRequired(this))
+ statement_.resultSetCommitting(false);
}
} finally {
markClosed();
connection_.CommitAndRollbackListeners_.remove(this);
}
- flowAutoCommitIfLastOpenMultipleResultSetWasJustClosed();
+// flowAutoCommitIfLastOpenMultipleResultSetWasJustClosed();
if (statement_.openOnClient_ && statement_.isCatalogQuery_) {
statement_.closeX();
}
@@ -411,33 +407,31 @@
resultSetMetaData_ = null;
}
- void flowCloseAndAutoCommitIfNotAutoCommitted() throws SqlException {
+ void flowCloseAndAutoCommit() throws SqlException {
agent_.beginWriteChain(statement_);
- writeCloseAndAutoCommitIfNotAutoCommitted();
+ writeCloseAndAutoCommit();
agent_.flow(statement_);
- readCloseAndAutoCommitIfNotAutoCommitted();
+ readCloseAndAutoCommit();
agent_.endReadChain();
}
- private void writeCloseAndAutoCommitIfNotAutoCommitted() throws
SqlException {
+ private void writeCloseAndAutoCommit() throws SqlException {
// set autoCommitted_ to false so commit will flow following
// close cursor if autoCommit is true.
autoCommitted_ = false;
if (generatedSection_ == null) { // none call statement result set case
writeCursorClose_(statement_.section_);
- writeAutoCommitIfNotAutoCommitted();
} else { // call statement result set(s) case
writeCursorClose_(generatedSection_);
}
+ if (statement_.isAutoCommitRequired(this))
+ statement_.resultSetCommitting(true);
}
- private void readCloseAndAutoCommitIfNotAutoCommitted() throws
SqlException {
- if (generatedSection_ == null) { // none call statement result set case
- readCursorClose_();
+ private void readCloseAndAutoCommit() throws SqlException {
+ readCursorClose_();
+ if (statement_.isAutoCommitRequired(this))
readAutoCommitIfNotAutoCommitted();
- } else { // call statement result set(s) case
- readCursorClose_();
- }
}
void writeClose() throws SqlException {
@@ -463,13 +457,6 @@
}
}
- void flowAutoCommitIfNotAutoCommitted() throws SqlException {
- if (generatedSection_ == null && connection_.autoCommit_ &&
!autoCommitted_) {
- connection_.flowAutoCommit();
- markAutoCommitted();
- }
- }
-
// precondition: transaction state allows for auto commit to generate flow
private void writeAutoCommitIfNotAutoCommitted() throws SqlException {
if (connection_.autoCommit_ && !autoCommitted_) {
@@ -480,29 +467,10 @@
private void readAutoCommitIfNotAutoCommitted() throws SqlException {
if (connection_.autoCommit_ && !autoCommitted_) {
connection_.readAutoCommit();
- markAutoCommitted();
+ connection_.markAutoCommitted();
}
}
- private void flowAutoCommitIfLastOpenMultipleResultSetWasJustClosed()
throws SqlException {
- // After this call, the generatedSection_ is reset to null to avoid
repeating the commit.
- if (generatedSection_ != null && statement_ != null &&
statement_.resultSetList_ != null) {
- int count = 0;
- for (int i = 0; i < statement_.resultSetList_.length; i++) {
- if (statement_.resultSetList_[i] == null) {
- count++;
- }
- }
- if (count == statement_.resultSetList_.length) {
- if (connection_.autoCommit_ && !autoCommitted_) {
- connection_.flowAutoCommit();
- markAutoCommitted();
- }
- }
- }
- generatedSection_ = null; // this is prevent a subsequent close() call
from doing another autocommit.
- }
-
public boolean wasNull() throws SqlException {
if (agent_.loggingEnabled()) {
@@ -2947,7 +2915,7 @@
public void completeLocalCommit(java.util.Iterator listenerIterator) {
cursorUnpositionedOnServer_ = true;
- markAutoCommitted();
+ connection_.markAutoCommitted();
if (!cursorHold_) {
// only non-held cursors need to be closed at commit
markClosed();
@@ -2959,7 +2927,7 @@
}
public void completeLocalRollback(java.util.Iterator listenerIterator) {
- markAutoCommitted();
+ connection_.markAutoCommitted();
// all cursors need to be closed at rollback
markClosed();
nullOutReferenceInStatement();
@@ -3028,7 +2996,7 @@
// the result set now.
public void setAutoCommitStateForSingletonCursors() {
if (connection_.autoCommit_) {
- markAutoCommitted();
+ connection_.markAutoCommitted();
}
}
@@ -3164,7 +3132,7 @@
throw new SqlException(agent_.logWriter_, "No updateXXX issued on
this row.");
}
preparedStatementForUpdate_ =
-
statement_.connection_.preparePositionedUpdateStatement(updateString,
+ connection_.preparePositionedUpdateStatement(updateString,
statement_.section_.getPositionedUpdateSection());
}
@@ -3174,7 +3142,7 @@
String deleteString = buildDeleteString();
preparedStatementForDelete_ =
-
statement_.connection_.preparePositionedUpdateStatement(deleteString,
+ connection_.preparePositionedUpdateStatement(deleteString,
statement_.section_.getPositionedUpdateSection()); //
update section
}
@@ -3856,18 +3824,13 @@
}
}
}
+
+ /**
+ * For debug purposes
+ *
+ * @return Whether this statement has been auto-committed
+ */
+ public boolean isAutoCommitted() {
+ return autoCommitted_;
+ }
}
-
-
-
-
-
-
-
-
-
-
-
-
-
-
