db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philip Wilder (JIRA)" <derby-...@db.apache.org>
Subject [jira] Commented: (DERBY-213) ResultSet.next() after last row of FORWARD_ONLY cursor throws an SQL Exception with Network Server
Date Fri, 17 Jun 2005 20:27:18 GMT
    [ 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


Mime
View raw message