You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by Daniel John Debrunner <dj...@apache.org> on 2008/02/14 18:10:24 UTC

Re: svn commit: r627673 - /db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/LangProcedureTest.java

mamta@apache.org wrote:
> Author: mamta
> Date: Wed Feb 13 22:24:50 2008
> New Revision: 627673
> 
> URL: http://svn.apache.org/viewvc?rev=627673&view=rev
> Log:
> DERBY-3304 and DERBY-3414
> +            s
> +            .execute("create procedure procWithRollback(p1 int) parameter style JAVA READS SQL DATA dynamic result sets 1 language java external name 'org.apache.derbyTesting.functionTests.tests.lang.LangProcedureTest.rollbackInsideProc'");
> +            drs1 = prepareCall("CALL procWithRollback(3)");
> +            drs1.execute();
> +            rs = drs1.getResultSet();

This fixture never asserts that the procedure does not return any result 
sets. It fetches rs, but never does anything with it. Instead of 
fetching rs, it should be using:
   JDBC.assertNoMoreResults(s);

There's a chance though that it may require DERBY-3404 to be fixed first.

[Note a procedure does not return closed result sets]

Dan.

Re: conn.rollback() in a procedure should not close the CallStatementResultSet - WAS Re: svn commit: r627673 - /db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/LangProcedureTest.java

Posted by Mamta Satoor <ms...@gmail.com>.
I will look at special handling of resultsets that do not return rows
to stay open at the time of rollback unless the rollback is caused by
an exception, in which case, we want to close all the resulsets
whether they return rows or not.

Mamta

On 2/15/08, Daniel John Debrunner <dj...@apache.org> wrote:
> Mamta Satoor wrote:
> > I will soon make a checkin for the resultset from the procedure which
> > shows that in trunk, after checkin 602991 for DERBY-1585, a procedure
> > does not return a resultset if there was a rollbck inside the
> > procedure with resultset creation before rollback.
>
> Thanks for finding the commit that caused it Mamta. Having made that
> change I had to look into why it changed the behaviour as that was not a
> intended part of the fix.
>
> DERBY-1585 fixed the issue that unprocessed dynamic result sets (e.g. in
> a trigger) were not closed until gc, leading to an error trying to drop
> a table due to an open result set.
>
> The fix (602991) changed it so that at the close of
> CallStatementResultSet any unprocessed dynamic result sets were closed.
> Thus the order is for a normal execution is:
>
>    callStmtResultSet.open();
>       calls the Java method for the procedure
>            method creates & returns JDBC ResultSets
>    embed statement processes dynamic result sets
>    callStmtResultSet.close();
>        no dynamic results to close since embed statement handled them
>
> (and for a trigger embed statement is not involved so the
> callStmtResultSet.close closes any dynamic result sets)
>
> Now if the procedure's method calls rollback() then this is the order:
>
> callStmtResultSet.open();
>       calls the Java method for the procedure
>            method creates & sets JDBC ResultSets in parameter arrays
>            rollback called()
>                callStmtResultSet.close()
>                    close all dynamic result sets <<<< NEW (DERBY-1585)
>       embed statement processes dynamic result sets
>       callStmtResultSet.close();
>        no dynamic results to close since embed statement handled them
>
> So the change occurred because the rollback caused the close of the
> CallStatementResultSet to be called which now closes all outstanding
> dynamic result sets and clears all references to them, making them
> invisible to any following embed statement processing, resulting in no
> dynamic results being returned.
>
> Prior to 602991 even though the underlying language result of any
> dynamic jdbc result set was closed, the jdbc result set did not appear
> closed (actually due to something similar to DERBY-3404), thus embed
> statement processed it as an open result set and returned it. Any use on
> it failed (it was closed) as a different mechanism was used to check it
> was closed.
>
> So I understand why DERBY-1585 (602991) changed the behaviour but I
> don't think the complete logic is correct. Much like a commit in a
> procedure was incorrectly closing the CallStatementResultSet (recently
> fixed by Mamta) I believe the rollback should not be closing the
> CallStatementResultSet. It's still being used and thus should remain
> open. Now a rollback due to an exception that causes the CALL statement
> to throw an exception should close the CallStatementResultSet, but if
> the CALL statement is returning then its CallStatementResultSet should
> not be closed.
>
> Dan.
>

conn.rollback() in a procedure should not close the CallStatementResultSet - WAS Re: svn commit: r627673 - /db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/LangProcedureTest.java

Posted by Daniel John Debrunner <dj...@apache.org>.
Mamta Satoor wrote:
> I will soon make a checkin for the resultset from the procedure which
> shows that in trunk, after checkin 602991 for DERBY-1585, a procedure
> does not return a resultset if there was a rollbck inside the
> procedure with resultset creation before rollback. 

Thanks for finding the commit that caused it Mamta. Having made that 
change I had to look into why it changed the behaviour as that was not a 
intended part of the fix.

DERBY-1585 fixed the issue that unprocessed dynamic result sets (e.g. in 
a trigger) were not closed until gc, leading to an error trying to drop 
a table due to an open result set.

The fix (602991) changed it so that at the close of 
CallStatementResultSet any unprocessed dynamic result sets were closed. 
Thus the order is for a normal execution is:

    callStmtResultSet.open();
       calls the Java method for the procedure
            method creates & returns JDBC ResultSets
    embed statement processes dynamic result sets
    callStmtResultSet.close();
        no dynamic results to close since embed statement handled them

(and for a trigger embed statement is not involved so the 
callStmtResultSet.close closes any dynamic result sets)

Now if the procedure's method calls rollback() then this is the order:

callStmtResultSet.open();
       calls the Java method for the procedure
            method creates & sets JDBC ResultSets in parameter arrays
            rollback called()
                callStmtResultSet.close()
                    close all dynamic result sets <<<< NEW (DERBY-1585)
       embed statement processes dynamic result sets
       callStmtResultSet.close();
        no dynamic results to close since embed statement handled them

So the change occurred because the rollback caused the close of the 
CallStatementResultSet to be called which now closes all outstanding 
dynamic result sets and clears all references to them, making them 
invisible to any following embed statement processing, resulting in no 
dynamic results being returned.

Prior to 602991 even though the underlying language result of any 
dynamic jdbc result set was closed, the jdbc result set did not appear 
closed (actually due to something similar to DERBY-3404), thus embed 
statement processed it as an open result set and returned it. Any use on 
it failed (it was closed) as a different mechanism was used to check it 
was closed.

So I understand why DERBY-1585 (602991) changed the behaviour but I 
don't think the complete logic is correct. Much like a commit in a 
procedure was incorrectly closing the CallStatementResultSet (recently 
fixed by Mamta) I believe the rollback should not be closing the 
CallStatementResultSet. It's still being used and thus should remain 
open. Now a rollback due to an exception that causes the CALL statement 
to throw an exception should close the CallStatementResultSet, but if 
the CALL statement is returning then its CallStatementResultSet should 
not be closed.

Dan.

Re: svn commit: r627673 - /db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/LangProcedureTest.java

Posted by Mamta Satoor <ms...@gmail.com>.
I will soon make a checkin for the resultset from the procedure which
shows that in trunk, after checkin 602991 for DERBY-1585, a procedure
does not return a resultset if there was a rollbck inside the
procedure with resultset creation before rollback. This behavior is
different than what happens in 10.2 codeline. In 10.2, a procedure
will return a *closed* resultset if there was a rollback inside the
procedure. From Dan's earlier comment in this thread([Note a procedure
does not return closed result sets]), it appears that trunk is
behaving correctly and 10.2's behavior was incorrect.

Mamta

On 2/14/08, Mamta Satoor <ms...@gmail.com> wrote:
> Dan, you must have read my mind about dynamic resultset in my test
> case. I didn't on purpose add anything for that resultset because I am
> getting ready to file a jira entry for dynamic resultset and rollback
> inside java procedure. A rollback inside proc in 10.1 returns a closed
> dynamic resultset but the same thing in trunk returns a null object
> for dynamic resultset. I think I am very close to finding what commit
> caused this behavior. I need little more time to confirm that and then
> file a jira entry which will modify the test I have added
>
> Mamta
> On 2/14/08, Daniel John Debrunner <dj...@apache.org> wrote:
> > mamta@apache.org wrote:
> > > Author: mamta
> > > Date: Wed Feb 13 22:24:50 2008
> > > New Revision: 627673
> > >
> > > URL: http://svn.apache.org/viewvc?rev=627673&view=rev
> > > Log:
> > > DERBY-3304 and DERBY-3414
> > > +            s
> > > +            .execute("create procedure procWithRollback(p1 int) parameter style JAVA READS SQL DATA dynamic result sets 1 language java external name 'org.apache.derbyTesting.functionTests.tests.lang.LangProcedureTest.rollbackInsideProc'");
> > > +            drs1 = prepareCall("CALL procWithRollback(3)");
> > > +            drs1.execute();
> > > +            rs = drs1.getResultSet();
> >
> > This fixture never asserts that the procedure does not return any result
> > sets. It fetches rs, but never does anything with it. Instead of
> > fetching rs, it should be using:
> >   JDBC.assertNoMoreResults(s);
> >
> > There's a chance though that it may require DERBY-3404 to be fixed first.
> >
> > [Note a procedure does not return closed result sets]
> >
> > Dan.
> >
>

Re: svn commit: r627673 - /db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/LangProcedureTest.java

Posted by Mamta Satoor <ms...@gmail.com>.
Dan, you must have read my mind about dynamic resultset in my test
case. I didn't on purpose add anything for that resultset because I am
getting ready to file a jira entry for dynamic resultset and rollback
inside java procedure. A rollback inside proc in 10.1 returns a closed
dynamic resultset but the same thing in trunk returns a null object
for dynamic resultset. I think I am very close to finding what commit
caused this behavior. I need little more time to confirm that and then
file a jira entry which will modify the test I have added

Mamta
On 2/14/08, Daniel John Debrunner <dj...@apache.org> wrote:
> mamta@apache.org wrote:
> > Author: mamta
> > Date: Wed Feb 13 22:24:50 2008
> > New Revision: 627673
> >
> > URL: http://svn.apache.org/viewvc?rev=627673&view=rev
> > Log:
> > DERBY-3304 and DERBY-3414
> > +            s
> > +            .execute("create procedure procWithRollback(p1 int) parameter style JAVA READS SQL DATA dynamic result sets 1 language java external name 'org.apache.derbyTesting.functionTests.tests.lang.LangProcedureTest.rollbackInsideProc'");
> > +            drs1 = prepareCall("CALL procWithRollback(3)");
> > +            drs1.execute();
> > +            rs = drs1.getResultSet();
>
> This fixture never asserts that the procedure does not return any result
> sets. It fetches rs, but never does anything with it. Instead of
> fetching rs, it should be using:
>   JDBC.assertNoMoreResults(s);
>
> There's a chance though that it may require DERBY-3404 to be fixed first.
>
> [Note a procedure does not return closed result sets]
>
> Dan.
>