Derby
  1. Derby
  2. DERBY-2620

embedded throws SQLState 8003 (No current connection) on rs.next() on closed resultSet in test for DERBY-1025 in DataSourceTest

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 10.3.1.4
    • Fix Version/s: None
    • Component/s: JDBC
    • Urgency:
      Low
    • Issue & fix info:
      Repro attached

      Description

      The following code checking that a CLOSE_CURSORS_AT_COMMIT ResultSet is closed by xa_start throws the wrong exception for embedded, indicating that there is no current connection instead of the ResultSet being closed.
      Statement s4 = conn4.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY, ResultSet.CLOSE_CURSORS_AT_COMMIT);
      ResultSet rs4 = s4.executeQuery("select i from autocommitxastart");
      rs4.next();
      assertEquals(1, rs4.getInt(1));
      rs4.next();
      assertEquals(2, rs4.getInt(1));

      // XAResource().start should commit the transaction
      try

      { xac4.getXAResource().start(xid4a, XAResource.TMNOFLAGS); xac4.getXAResource().end(xid4a, XAResource.TMSUCCESS); }

      catch (XAException xae)

      { fail("unexpected XAException on xac4.getXAResource.start or end"); }

      catch (Exception e)

      { fail("unexpected Exception on xac4.getXAResource.start or end"); }

      // DERBY-1025.
      // With Embedded, this will give error: 08003 - No current connection
      // But with NetworkServer / DerbyNetClient, the transaction does not
      // appear to be closed, and we actually get a value.
      try

      { rs4.next(); rs4.getInt(1); fail ("expected an exception indicating resultset is closed."); }

      catch (SQLException sqle)

      { // Embedded gets 08003. if (usingDerbyNetClient()) assertSQLState("XCL16",sqle); }

        Activity

        Hide
        Kathey Marsden added a comment -

        I am unnassigning myself from this issue for now. I don't have a clear idea of the best solution.

        Show
        Kathey Marsden added a comment - I am unnassigning myself from this issue for now. I don't have a clear idea of the best solution.
        Hide
        Kathey Marsden added a comment -

        Currently, since xa end sets the application to null, so we get the 8003 (No connection) message if we try to use the resultset, Could you explain the logic behind setting the applicationConnection to null in XATransactionState.end()

        Show
        Kathey Marsden added a comment - Currently, since xa end sets the application to null, so we get the 8003 (No connection) message if we try to use the resultset, Could you explain the logic behind setting the applicationConnection to null in XATransactionState.end()
        Hide
        Daniel John Debrunner added a comment -

        > Question: 1) Should the ResultSet created before a global transaction be usable after a global transaction?

        For a ResultSet that is CLOSE_CURSORS_AT_COMMIT then no, since the switch to an global transaction from a local one will cause the local one to commit and thus close the ResultSet.

        For a held cursor then it should remain open but I don't know if Derby supports that.

        Show
        Daniel John Debrunner added a comment - > Question: 1) Should the ResultSet created before a global transaction be usable after a global transaction? For a ResultSet that is CLOSE_CURSORS_AT_COMMIT then no, since the switch to an global transaction from a local one will cause the local one to commit and thus close the ResultSet. For a held cursor then it should remain open but I don't know if Derby supports that.
        Hide
        Kathey Marsden added a comment -

        Interestingly, if I put a s4.getConnection() before the rs4.next() call, I get the expected error, so perhaps the correct solution is to reestablish a real connection on rs.next()

        Show
        Kathey Marsden added a comment - Interestingly, if I put a s4.getConnection() before the rs4.next() call, I get the expected error, so perhaps the correct solution is to reestablish a real connection on rs.next()
        Hide
        Kathey Marsden added a comment -

        The reason for the error is that xa_end sets the application connection to null.Then on the first operation outside of the global xact,for instance calling Connection.isClosed() or createStatement() it will make a call to
        BrokeredConnection.getRealConnection() which will create a new EmbedConnection if the realConnection is null. However for operations on existing resultSets we get the 8003 error because the ResultSet is still linked to the BrokeredConnection whose realConnection is still null.

        Question:
        1) Should the ResultSet created before a global transaction be usable after a global transaction?
        If not we could just change EmbedResultSet.checkExecIfClosed() to throw SQLState.LANG_RESULT_SET_NOT_OPEN if the connection is null. There are likely other places e.g. Statement objects where similar changes would need to be made.

        Show
        Kathey Marsden added a comment - The reason for the error is that xa_end sets the application connection to null.Then on the first operation outside of the global xact,for instance calling Connection.isClosed() or createStatement() it will make a call to BrokeredConnection.getRealConnection() which will create a new EmbedConnection if the realConnection is null. However for operations on existing resultSets we get the 8003 error because the ResultSet is still linked to the BrokeredConnection whose realConnection is still null. Question: 1) Should the ResultSet created before a global transaction be usable after a global transaction? If not we could just change EmbedResultSet.checkExecIfClosed() to throw SQLState.LANG_RESULT_SET_NOT_OPEN if the connection is null. There are likely other places e.g. Statement objects where similar changes would need to be made.
        Hide
        Myrna van Lunteren added a comment -

        There's probably no good reason for the first one, except maybe an attempt to roll past the offending pieces so I could convert the rest of the test.
        Probably no good reason to have the next() in the try/catch blocks; just that the original test had it constructed like this.
        But this is high-jacking this particular bug, these comments probably belong with the original test conversion...see DERBY-2492.

        Show
        Myrna van Lunteren added a comment - There's probably no good reason for the first one, except maybe an attempt to roll past the offending pieces so I could convert the rest of the test. Probably no good reason to have the next() in the try/catch blocks; just that the original test had it constructed like this. But this is high-jacking this particular bug, these comments probably belong with the original test conversion...see DERBY-2492 .
        Hide
        Daniel John Debrunner added a comment -

        I see the code above is from the DataSource test.
        Is there a reason for catching the exceptions (in the first block) and calling fail() in the catch block? The test will fail if the exceptions are thrown without a catch block and the junit runner will show a stack trace. With the fail it will not be clear which method caused the problem, or where the exception was actually thrown from.

        And in the second case, is it the next() or getInt() that is expected to fail? Putting two method calls that can throw the same exception when expecting one of them to fail is a way of hiding errors and being confusing to the test reader. E.g. if the next() is meant to succeed and the getInt() fail then the test would pass even if the next() throws an exception. If the next() is meant to fail() then what's the purpose of calling getInt()?

        Show
        Daniel John Debrunner added a comment - I see the code above is from the DataSource test. Is there a reason for catching the exceptions (in the first block) and calling fail() in the catch block? The test will fail if the exceptions are thrown without a catch block and the junit runner will show a stack trace. With the fail it will not be clear which method caused the problem, or where the exception was actually thrown from. And in the second case, is it the next() or getInt() that is expected to fail? Putting two method calls that can throw the same exception when expecting one of them to fail is a way of hiding errors and being confusing to the test reader. E.g. if the next() is meant to succeed and the getInt() fail then the test would pass even if the next() throws an exception. If the next() is meant to fail() then what's the purpose of calling getInt()?
        Hide
        Daniel John Debrunner added a comment -

        See the javadoc for EmbedConnection.applicationConnection.

        Show
        Daniel John Debrunner added a comment - See the javadoc for EmbedConnection.applicationConnection.
        Hide
        Kathey Marsden added a comment -

        I see this code where the error occurs.

        java.sql.Connection appConn = getEmbedConnection().getApplicationConnection();

        // Currently disconnected, i.e. a detached gobal transaction
        if (appConn == null)
        throw Util.noCurrentConnection();

        We are detatched from the global transaction, but this statement was never used.
        In this context getEmbedConnection() returns a valid connection, but getApplicationConnection() is null.
        What is the difference between the EmbedConnection and the ApplicationConnection?

        Show
        Kathey Marsden added a comment - I see this code where the error occurs. java.sql.Connection appConn = getEmbedConnection().getApplicationConnection(); // Currently disconnected, i.e. a detached gobal transaction if (appConn == null) throw Util.noCurrentConnection(); We are detatched from the global transaction, but this statement was never used. In this context getEmbedConnection() returns a valid connection, but getApplicationConnection() is null. What is the difference between the EmbedConnection and the ApplicationConnection?

          People

          • Assignee:
            Unassigned
            Reporter:
            Kathey Marsden
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development