Derby
  1. Derby
  2. DERBY-5161

Cannot rollback after syntax error in internal statement

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.7.1.1
    • Fix Version/s: 10.8.1.2
    • Component/s: SQL
    • Labels:
      None

      Description

      To reproduce, execute the statements below in ij. Can only be reproduced this way before DERBY-5157. I don't know how to reproduce it when that bug is fixed.

      ij version 10.7
      ij> connect 'jdbc:derby:db;create=true';
      ij> autocommit off;
      ij> create table t(x int);
      0 rows inserted/updated/deleted
      ij> alter table t add column """" int default 42;
      ERROR 42X01: Syntax error: Encountered "\"" at line 1, column 22.
      Issue the 'help' command for general information on IJ command syntax.
      Any unrecognized commands are treated as potential SQL commands and executed directly.
      Consult your DBMS server reference documentation for details of the SQL syntax supported by your server.
      ij> rollback;
      ERROR X0Y67: Cannot issue rollback in a nested connection when there is a pending operation in the parent connection.

      The error message implies that we've called rollback() on a nested transaction, whereas we're in fact called it on the parent transaction.

      Expected result: The rollback statement should abort the transaction without raising any errors.

      1. finally.diff
        0.7 kB
        Knut Anders Hatlen
      2. cleanupOnError.diff
        0.5 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open In Progress In Progress
          20m 40s 1 Knut Anders Hatlen 29/Mar/11 10:13
          In Progress In Progress Resolved Resolved
          1d 3h 1m 1 Knut Anders Hatlen 30/Mar/11 13:14
          Resolved Resolved Closed Closed
          91d 21h 6m 1 Knut Anders Hatlen 30/Jun/11 10:21
          Gavin made changes -
          Workflow jira [ 12608944 ] Default workflow, editable Closed status [ 12801090 ]
          Knut Anders Hatlen made changes -
          Link This issue is related to DERBY-5406 [ DERBY-5406 ]
          Knut Anders Hatlen made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Rick Hillegas made changes -
          Link This issue relates to DERBY-5280 [ DERBY-5280 ]
          Rick Hillegas made changes -
          Fix Version/s 10.8.1.2 [ 12316362 ]
          Fix Version/s 10.8.1.1 [ 12316356 ]
          Rick Hillegas made changes -
          Fix Version/s 10.8.1.1 [ 12316356 ]
          Fix Version/s 10.8.1.0 [ 12315561 ]
          Knut Anders Hatlen made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Fix Version/s 10.8.0.0 [ 12315561 ]
          Resolution Fixed [ 1 ]
          Hide
          Knut Anders Hatlen added a comment -

          All regression tests ran cleanly with the patch. Committed revision 1086920.

          Show
          Knut Anders Hatlen added a comment - All regression tests ran cleanly with the patch. Committed revision 1086920.
          Knut Anders Hatlen made changes -
          Attachment cleanupOnError.diff [ 12474878 ]
          Hide
          Knut Anders Hatlen added a comment -

          I ran the regression tests on the finally.diff patch and saw two failures in derbyall:

          lang/subquery.sql
          lang/subquery2.sql

          All diffs were similar to this one:

                          • Diff file derbyall/derbylang/subquery2.diff
              • Start: subquery2 jdk1.6.0_21 derbyall:derbylang 2011-03-29 14:03:32 ***
                1017 del
                < 2 dependencies found
                1017a1017
                > 12 dependencies found
                Test Failed.
              • End: subquery2 jdk1.6.0_21 derbyall:derbylang 2011-03-29 14:03:44 ***

          The problem appears to be that some dependencies aren't cleared when we pop the statement context after a syntax error. I think we should use StatementContext.cleanupOnError() instead of LanguageConnectionContext.popStatementContext() when an exception has been thrown. cleanupOnError() will call popStatementContext() after first clearing the dependencies that made subquery.sql and subquery2.sql fail.

          The attached patch cleanupOnError.diff changes GenericStatement.prepMinion() so that it uses cleanupOnError() in the exception case. subquery.sql and subquery2.sql pass with that change, and the patch still fixes the rollback error in the bug repro.

          Show
          Knut Anders Hatlen added a comment - I ran the regression tests on the finally.diff patch and saw two failures in derbyall: lang/subquery.sql lang/subquery2.sql All diffs were similar to this one: Diff file derbyall/derbylang/subquery2.diff Start: subquery2 jdk1.6.0_21 derbyall:derbylang 2011-03-29 14:03:32 *** 1017 del < 2 dependencies found 1017a1017 > 12 dependencies found Test Failed. End: subquery2 jdk1.6.0_21 derbyall:derbylang 2011-03-29 14:03:44 *** The problem appears to be that some dependencies aren't cleared when we pop the statement context after a syntax error. I think we should use StatementContext.cleanupOnError() instead of LanguageConnectionContext.popStatementContext() when an exception has been thrown. cleanupOnError() will call popStatementContext() after first clearing the dependencies that made subquery.sql and subquery2.sql fail. The attached patch cleanupOnError.diff changes GenericStatement.prepMinion() so that it uses cleanupOnError() in the exception case. subquery.sql and subquery2.sql pass with that change, and the patch still fixes the rollback error in the bug repro.
          Hide
          Dag H. Wanvik added a comment -

          So, this explains the hang I saw in DERBY-5157 with the new test cases present and the code changes backed out. Looks like a good fix to me if regressions pass. Although we should not normally see syntax errors in internal statements, its good that
          the error handling is correct if it ever happens.

          +1

          Show
          Dag H. Wanvik added a comment - So, this explains the hang I saw in DERBY-5157 with the new test cases present and the code changes backed out. Looks like a good fix to me if regressions pass. Although we should not normally see syntax errors in internal statements, its good that the error handling is correct if it ever happens. +1
          Knut Anders Hatlen made changes -
          Link This issue relates to DERBY-5157 [ DERBY-5157 ]
          Knut Anders Hatlen made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Knut Anders Hatlen made changes -
          Assignee Knut Anders Hatlen [ knutanders ]
          Knut Anders Hatlen made changes -
          Field Original Value New Value
          Attachment finally.diff [ 12474861 ]
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a patch that moves popStatementContext() into a finally clause. Now the repro doesn't fail when invoking rollback:

          ij version 10.8
          ij> connect 'jdbc:derby:db;create=true';
          ij> autocommit off;
          ij> create table t(x int);
          0 rows inserted/updated/deleted
          ij> alter table t add column """" int default 42;
          ERROR 42X01: Syntax error: Encountered "\"" at line 1, column 22.
          Issue the 'help' command for general information on IJ command syntax.
          Any unrecognized commands are treated as potential SQL commands and executed directly.
          Consult your DBMS server reference documentation for details of the SQL syntax supported by your server.
          ij> rollback;
          ij>

          I haven't run any other tests yet. No regression tests have been added since I don't know how to reproduce the problem without backing out DERBY-5157.

          Show
          Knut Anders Hatlen added a comment - Attaching a patch that moves popStatementContext() into a finally clause. Now the repro doesn't fail when invoking rollback: ij version 10.8 ij> connect 'jdbc:derby:db;create=true'; ij> autocommit off; ij> create table t(x int); 0 rows inserted/updated/deleted ij> alter table t add column """" int default 42; ERROR 42X01: Syntax error: Encountered "\"" at line 1, column 22. Issue the 'help' command for general information on IJ command syntax. Any unrecognized commands are treated as potential SQL commands and executed directly. Consult your DBMS server reference documentation for details of the SQL syntax supported by your server. ij> rollback; ij> I haven't run any other tests yet. No regression tests have been added since I don't know how to reproduce the problem without backing out DERBY-5157 .
          Hide
          Knut Anders Hatlen added a comment -

          The rollback exception is raised by this code in GenericLanguageConnectionContext.doRollback():

          StatementContext statementContext = getStatementContext();
          if (requestedByUser &&
          (statementContext != null) &&
          statementContext.inUse() &&
          statementContext.isAtomic())

          { throw StandardException.newException(SQLState.LANG_NO_ROLLBACK_IN_NESTED_CONNECTION); }

          So it seems the problem is that the statement context wasn't popped from the stack when the internal statement failed.

          Looking at the code in GenericStatement.prepMinion(), in which the failing call to parseStatement() is invoked, there is a try/finally block that ensures the compiler context is popped from the stack, but the statement context isn't popped until after the finally clause, and may therefore be forgotten in the case of a failure. I think we may want to move the call to popStatementContext() into the finally clause.

          Show
          Knut Anders Hatlen added a comment - The rollback exception is raised by this code in GenericLanguageConnectionContext.doRollback(): StatementContext statementContext = getStatementContext(); if (requestedByUser && (statementContext != null) && statementContext.inUse() && statementContext.isAtomic()) { throw StandardException.newException(SQLState.LANG_NO_ROLLBACK_IN_NESTED_CONNECTION); } So it seems the problem is that the statement context wasn't popped from the stack when the internal statement failed. Looking at the code in GenericStatement.prepMinion(), in which the failing call to parseStatement() is invoked, there is a try/finally block that ensures the compiler context is popped from the stack, but the statement context isn't popped until after the finally clause, and may therefore be forgotten in the case of a failure. I think we may want to move the call to popStatementContext() into the finally clause.
          Knut Anders Hatlen created issue -

            People

            • Assignee:
              Knut Anders Hatlen
              Reporter:
              Knut Anders Hatlen
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development