[
http://issues.apache.org/jira/browse/DERBY-213?page=comments#action_12313920 ]
Philip Wilder commented on DERBY-213:
-------------------------------------
Proposed Changes:
The following are a series of changes I propose to rectify the problem of
Derby-213 and bring the client code more in line with Embedded. Initial tests
look promising, but a full series of tests have not yet been developed. Any and
all feedback is appreciated, sorry for the length.
####################
client.am.Connection
####################
In the method flowAutoCommit()
-Change
The return value to boolean on successful flow which returns true on a
successful flow.
###################
client.am.Statement
###################
- Create new methods:
/**
* Convenience method for resultSetCommitting(ResultSet, boolean)
*
* @see Statement#resultSetCommitting(ResultSet, boolean)
* @param closingRS The ResultSet to be closed
* @throws SqlException
*/
public void resultSetCommitting(ResultSet closingRS) throws SqlException {
resultSetCommitting(closingRS, false);
}
/**
* 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(ResultSet closingRS, boolean writeChain)
throws SqlException {
// If the Connection is not in auto commit then this statement
completion
// cannot cause a commit.
if (!connection_.autoCommit_ || closingRS.autoCommitted_)
return;
// If we have multiple results, see if there is another result
set open.
// If so, then no commit. The last result set to close will
close the statement.
if (resultSetList_ != null) {
for (int i = 0; i < resultSetList_.length; i++) {
ResultSet crs = resultSetList_[i];
if (crs == null)
continue;
if (!crs.openOnClient_)
continue;
if (crs == closingRS)
continue;
// at least one still open so no commit now.
return;
}
}
if (writeChain) {
connection_.writeAutoCommit();
} else {
if (connection_.flowAutoCommit())
closingRS.markAutoCommitted();
}
}
###################
client.am.ResultSet
###################
In the method nextX()
- Change
if ((!isValidCursorPosition_ && cursor_ != null) || (statement_.maxRows_ > 0 &&
cursor_.rowsRead_ > statement_.maxRows_)) {
isValidCursorPosition_ = false;
...
to
if (statement_.maxRows_ > 0 && cursor_.rowsRead_ > statement_.maxRows_) {
isValidCursorPosition_ = false;
}
This provides a simplification of the logic as there is no need to set
isValidCursorPosition_ = false when
!isValidCursorPosition_ == true
-Remove
A trailing '}' to insure that braces match
- Change
if (!openOnServer_) {
to
if (!isValidCursorPosition_ && !openOnServer_) {
Follows from the first change.
-Remove
try {
closeX(); // the auto commit logic is in closeX()
} catch (SqlException sqle) {
sqlException = Utils.accumulateSQLException(sqle, sqlException);
}
There should be no reference to closeX() in the ResultSet.nextX() method. If
the ResultSetHoldability == ResultSet.CLOSE_CURSORS_AT_COMMIT then the
ResultSet should close properly anyway.
-Remove from
if (isValidCursorPosition_) {
updateColumnInfoFromCache();
// check if there is a non-null SQLCA for the current row for rowset cursors
checkRowsetSqlca();
if (isBeforeFirst_) {
isFirst_ = true;
}
isBeforeFirst_ = false;
} else {
isFirst_ = false;
return isValidCursorPosition_;
}
the line
return isValidCursorPosition_;
from the else clause. For reasons that shall become apparent.
-Remove
if (!openOnClient_) {
isValidCursorPosition_ = false;
} else
I'm confident that this can't actually happen because of the
checkForClosedResultSet() call earlier
-Add (immediately before the Return)
//An invalid cursor position is synonymous with a completed
//ResultSet thus we should make the call to
//statement_.resultSetComitting(this) for both FORWARD_ONLY
//and SCROLLABLE as per embedded behaviour
if (!isValidCursorPosition_)
statement_.resultSetCommitting(this);
In the Method writeCloseAndAutoCommitIfNotAutoCommitted()
- Change one of two things
* Remove autoCommitted_ = false;
This strikes me as misleading for a method called
writeCloseAndAutoCommit*IfNotAutoCommitted*()
* Change the name to writeCloseAndAutoCommit()
Acknowledges that a auto commit is necessary after a close regardless of its
autocommit status and change the name to match the functionality. In conjuction
with this change it would be necessary to change the method named
flowCloseAndAutoCommitIfNotAutoCommitted()
to
flowCloseAndAutoCommit()
and
readCloseAndAutoCommitIfNotAutoCommitted()
to
readCloseAndAutoCommit()
-Remove
writeAutoCommitIfNotAutoCommitted();
-Add
statement_.resultSetCommitting(this, true);
outside the if block.
These two methods do similar things but the statement_.resultSetCommitting()
method:
a) Places the commit logic within the Statement class which is where it belongs
b) Checks to see if there are any open ResultSets before committing which I
understand may not be necessary for the moment but could be useful for future
compatibility. We do not wish to set the autoCommitted_ value to true because
we are part of a write read chain and autoCommited_ will be set to true at the
end.
In the method closeX()
-Change
if (openOnServer_) {
flowCloseAndAutoCommitIfNotAutoCommitted();
} else {
flowAutoCommitIfNotAutoCommitted() // in case of early close
}
to
if (openOnServer_) {
flowCloseAndAutoCommitIfNotAutoCommitted();
} else {
statement_.resultSetCommitting(this);
}
-Remove
flowAutoCommitIfLastOpenMultipleResultSetWasJustClosed();
The statement_.resultSetCommitting(this); already checks for multiple
ResultSets and commits only if the the only open ResultSet is the ResultSet
which called the method.
Remove the method flowAutoCommitIfNotAutoCommitted()
Remove the method flowAutoCommitIfLastOpenMultipleResultSetWasJustClosed()
Add a method
/**
* Getter used exclusively for testing purposes.
*
* @return autoCommited_
*/
public boolean isAutoCommitted() {
return autoCommitted_;
}
####################
impl.jdbc.EmbedResultSet
####################
-Add
Instance variable
boolean isAutoCommitted
for testing Purposes
-Add method
/**
* For testing purposes only. Returns whether this ResultSet has been
* automatically committed
*
* @return isAutoCommitted.
*/
public boolean isAutoCommitted() {
return isAutoCommitted;
}
####################
impl.jdbc.EmbedStatement
####################
At the bottom of the method
- Add
//For testing
closingLRS.isAutoCommitted = true;
#####################
derbyTesting.functionTests.tests.jdbcapi.resultTest
#####################
The tests to insure the code is operating are somewhat generally simple in
their format yet difficult in that there are a number of them and there is
little that can be done in the way of code reuse. I propose a series of tests
akin to this format:
private static void runTests(Connection conn) throws SQLException {
Statement s = conn.createStatement();
ResultSet rs = s.executeQuery("select tablename from
sys.systables " +
"where tablename = '" + tableName.toUpperCase()
+ "'");
if (rs.next()) {
rs.close();
s.execute("delete from " + tableName);
} else {
rs.close();
s.execute("create table " + tableName + " (num int)");
}
s.execute("insert into " + tableName + " values (1)");
s.execute("insert into " + tableName + " values (2)");
boolean commitVal = conn.getAutoCommit();
test1(conn); //test1 through testX
conn.setAutoCommit(commitVal);
s.execute("drop table " + tableName);
}
/**
* Tests
* - autoCommit == true
* - resultSetType == TYPE_FORWARD_ONLY
* - holdability == HOLD_CURSORS_OVER_COMMIT
* - limit returned ResultSets == false
*
* @param conn
* @throws SQLException
*/
private static void test1(Connection conn) throws SQLException {
System.out.print("test1: ");
conn.setAutoCommit(true);
Statement s = conn.createStatement(ResultSet.TYPE_FORWARD_ONLY,
ResultSet.CONCUR_READ_ONLY, ResultSet.HOLD_CURSORS_OVER_COMMIT);
ResultSetWrapper rsw = new
ResultSetWrapper(s.executeQuery("select * from " + tableName));
int count = 0;
while (rsw.next()) count++;
if (!rsw.isAutoCommitted()) {
System.out.println("Failed. Not committed properly");
}
try {
if (rsw.next())
System.out.println("Failed. ResultSet returned
true after last.");
if (count == 2)
System.out.println("Succeeded");
else
System.out.println("Failed. Unexpected row
count of " + count);
rsw.close();
} catch (SQLException e) {
System.out.println("Failed. Unexpected exception: ");
e.printStackTrace(System.out);
}
s.close();
}
/**
* This class provides me with a simple way of check if a ResultSet
* has been auto committed regardless of the underlying ResultSet
* being used
*
* @author Philip Wilder
*/
private class ResultSetWrapper {
public ResultSetWrapper(ResultSet rs) {
if (rs instanceof org.apache.derby.client.am.ResultSet)
clientRS =
(org.apache.derby.client.am.ResultSet)rs;
else if (rs instanceof
org.apache.derby.impl.jdbc.EmbedResultSet)
embedRS =
(org.apache.derby.impl.jdbc.EmbedResultSet)rs;
this.rs = rs;
}
public boolean next() throws SQLException {
return rs.next();
}
public void close() throws SQLException {
rs.close();
}
public String getAutoCommittedString() {
if (clientRS != null)
return clientRS.isAutoCommitted() ? "true" :
"false";
else if (embedRS != null)
return embedRS.isAutoCommitted() ? "true" :
"false";
else
return "Unknown";
}
public boolean isAutoCommitted() {
if (clientRS != null)
return clientRS.isAutoCommitted();
else if (embedRS != null)
return embedRS.isAutoCommitted();
else
return false;
}
private org.apache.derby.client.am.ResultSet clientRS = null;
private org.apache.derby.impl.jdbc.EmbedResultSet embedRS =
null;
private ResultSet rs = null;
}
private String tableName = "commitTestTable";
> ResultSet.next() after last row of FORWARD_ONLY cursor throws an SQL
> Exception with Network Server
> --------------------------------------------------------------------------------------------------
>
> Key: DERBY-213
> URL: http://issues.apache.org/jira/browse/DERBY-213
> Project: Derby
> Type: Bug
> Components: Network Client
> Versions: 10.1.0.0
> Reporter: Kathey Marsden
> Assignee: Philip Wilder
> Attachments: Client.java, Create.java, DERBY-213_6_13_2005.txt,
> DERBY-213_6_9_2005.txt, DERBY-213_irc_6_3_2005, DERBY-213_irc_6_7_2005.txt,
> DERBY-213_irc_6_8_2005, IRCTranscript_June2_2005.txt, ResultSet Outline.pdf,
> Server.java, resultset.java
>
> Network Server closes the result set if ResultSet.next() is
> called after the last row of the result set. The test code
> below throws the following exception.
> SQLState: null
> Severity: -99999
> Message: Invalid operation: result set closed
> com.ibm.db2.jcc.am.SqlException: Invalid operation: result set
> closed
> at
> com.ibm.db2.jcc.am.ResultSet.checkForClosedResultSet(ResultSet.j
> ava:3419)
> at
> com.ibm.db2.jcc.am.ResultSet.nextX(ResultSet.java:290)
> at
> com.ibm.db2.jcc.am.ResultSet.next(ResultSet.java:277)
> at AfterLast.test(AfterLast.java:75)
> at AfterLast.main(AfterLast.java:32)
> stmt.executeUpdate("CREATE TABLE TAB ( I INT)");
> stmt.executeUpdate("INSERT INTO TAB VALUES(1)");
> stmt.executeUpdate("INSERT INTO TAB VALUES(2)");
> String sql ="SELECT * from tab";
> ps = conn.prepareStatement(sql);
> ResultSet rs = ps.executeQuery();
> System.out.println(sql);
> while (rs.next())
> System.out.println(rs.getInt(1));
> try {
> System.out.println("one more next");
> rs.next();
> }
> catch (Exception e)
> {
> System.out.println("FAIL: next should return false not throw
> exception");
> e.printStackTrace();
> }
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
