Derby
  1. Derby
  2. DERBY-5163

[patch] fix up sql cleanup handling

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 10.7.1.1
    • Fix Version/s: 10.8.3.3, 10.9.1.0
    • Component/s: Tools
    • Labels:
      None
    • Issue & fix info:
      Patch Available

      Description

      When a sql exception occurs, don't allow potential cascading sql problems when closing sql objects from masking the original cause of the exception.
      Also make sure statements get closed.

        Issue Links

          Activity

          Dave Brosius created issue -
          Dave Brosius made changes -
          Field Original Value New Value
          Attachment sql_cleanup_fixes.diff [ 12474951 ]
          Hide
          Lily Wei added a comment -

          Thanks Dave for the patch. I run some tests with this patch. When I run 'java -Dderby.tests.trace=true -Dderby.tests.debug=true junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.tools.ToolScripts', I got 4 failures.
          Here is some stacktrace:
          1) ij_show_roles_dbo(org.apache.derbyTesting.functionTests.tests.tools.ToolScrip
          ts)junit.framework.ComparisonFailure: Output at line 3 expected:<ROLEID
          > but was:<ERROR XJ012: 'Statement' already closed.>
          at org.apache.derbyTesting.functionTests.util.CanonTestCase.compareCanon
          (CanonTestCase.java:109)
          at org.apache.derbyTesting.functionTests.util.ScriptTestCase.runTest(Scr
          iptTestCase.java:201)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:
          112)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          at junit.extensions.TestSetup.run(TestSetup.java:23)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57
          )
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          at junit.extensions.TestSetup.run(TestSetup.java:23)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57
          )
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)

          Show
          Lily Wei added a comment - Thanks Dave for the patch. I run some tests with this patch. When I run 'java -Dderby.tests.trace=true -Dderby.tests.debug=true junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.tools.ToolScripts', I got 4 failures. Here is some stacktrace: 1) ij_show_roles_dbo(org.apache.derbyTesting.functionTests.tests.tools.ToolScrip ts)junit.framework.ComparisonFailure: Output at line 3 expected:<ROLEID > but was:<ERROR XJ012: 'Statement' already closed.> at org.apache.derbyTesting.functionTests.util.CanonTestCase.compareCanon (CanonTestCase.java:109) at org.apache.derbyTesting.functionTests.util.ScriptTestCase.runTest(Scr iptTestCase.java:201) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java: 112) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19) at junit.extensions.TestSetup.run(TestSetup.java:23) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57 ) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19) at junit.extensions.TestSetup.run(TestSetup.java:23) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57 ) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          Rick Hillegas made changes -
          Fix Version/s 10.8.1.1 [ 12316356 ]
          Fix Version/s 10.8.1.0 [ 12315561 ]
          Hide
          Kathey Marsden added a comment -

          Unmarking patch available as Lily discovered some test failures. Dave, have you seen these in your testing? Any idea what the issue might be?

          Show
          Kathey Marsden added a comment - Unmarking patch available as Lily discovered some test failures. Dave, have you seen these in your testing? Any idea what the issue might be?
          Kathey Marsden made changes -
          Issue & fix info [Patch Available]
          Dave Brosius made changes -
          Attachment sql_cleanup_fixes.diff [ 12474951 ]
          Hide
          Dave Brosius added a comment -

          Cleanup patch so that we don't close statements when resultsets from those statements are still active.

          Show
          Dave Brosius added a comment - Cleanup patch so that we don't close statements when resultsets from those statements are still active.
          Dave Brosius made changes -
          Attachment sql_cleanup_fixes.diff [ 12476515 ]
          Hide
          Kathey Marsden added a comment -

          Thank you Dave for the quick response. Could you run suites.All and derbyall if you haven't already to verify there are no additional issues with the regression tests? It is always good to post the test results with patches.

          Show
          Kathey Marsden added a comment - Thank you Dave for the quick response. Could you run suites.All and derbyall if you haven't already to verify there are no additional issues with the regression tests? It is always good to post the test results with patches.
          Hide
          Lily Wei added a comment -

          Thanks Dave and Kathey. When I post a patch, I usually run suites.All and derbyall using the following script:
          java -client -Dderby.tests.trace=true -XX:MaxPermSize=192M -Xmx1024M -Xms512M -D
          derbyTesting.oldReleasePath=c:/derby/oldrelease/jars junit.textui.TestRunner org
          .apache.derbyTesting.functionTests.suites.AllPackages 2>&1 | tee rjall.out

          and

          java org.apache.derbyTesting.functionTests.harness.RunSuite derbyall 2>&1 | tee
          djall.out

          Show
          Lily Wei added a comment - Thanks Dave and Kathey. When I post a patch, I usually run suites.All and derbyall using the following script: java -client -Dderby.tests.trace=true -XX:MaxPermSize=192M -Xmx1024M -Xms512M -D derbyTesting.oldReleasePath=c:/derby/oldrelease/jars junit.textui.TestRunner org .apache.derbyTesting.functionTests.suites.AllPackages 2>&1 | tee rjall.out and java org.apache.derbyTesting.functionTests.harness.RunSuite derbyall 2>&1 | tee djall.out
          Hide
          Lily Wei added a comment -

          FYI. I run Suites.all with the new patch and everything pass for me with the patch.

          Show
          Lily Wei added a comment - FYI. I run Suites.all with the new patch and everything pass for me with the patch.
          Hide
          Dave Brosius added a comment -

          Thanks Lily, Kathey - i have a bear of a time getting the tests to all pass before these changes. I'm thinking the errors are memory related currently, hopefully i've got the special sauce going now, based on Lily's comments.

          Show
          Dave Brosius added a comment - Thanks Lily, Kathey - i have a bear of a time getting the tests to all pass before these changes. I'm thinking the errors are memory related currently, hopefully i've got the special sauce going now, based on Lily's comments.
          Rick Hillegas made changes -
          Fix Version/s 10.8.1.2 [ 12316362 ]
          Fix Version/s 10.8.1.1 [ 12316356 ]
          Dave Brosius made changes -
          Attachment sql_cleanup_fixes.diff [ 12476515 ]
          Hide
          Dave Brosius added a comment -

          ran the following

          java -classpath tools/java/junit.jar:tools/java/jakarta-oro-2.0.8.jar:jars/sane/derbyTesting.jar:jars/sane/derby.jar:jars/sane/derbyTools.jar:jars/sane/derbyTools.jar:jars/sane/derbyrun.jar:jars/sane/derbynet.jar:jars/sane/derbyclient.jar -Xmx800m -XX:MaxPermSize=512m junit.textui.TestRunner org.apache.derbyTesting.functionTests.suites.All

          got no errors

          Time: 21,886.483

          OK (13459 tests)

          Show
          Dave Brosius added a comment - ran the following java -classpath tools/java/junit.jar:tools/java/jakarta-oro-2.0.8.jar:jars/sane/derbyTesting.jar:jars/sane/derby.jar:jars/sane/derbyTools.jar:jars/sane/derbyTools.jar:jars/sane/derbyrun.jar:jars/sane/derbynet.jar:jars/sane/derbyclient.jar -Xmx800m -XX:MaxPermSize=512m junit.textui.TestRunner org.apache.derbyTesting.functionTests.suites.All got no errors Time: 21,886.483 OK (13459 tests)
          Dave Brosius made changes -
          Attachment sql_cleanup_fixes.diff [ 12476830 ]
          Hide
          Kathey Marsden added a comment - - edited

          I am sorry for the delay in responding. If you think the new patch is read for commit can you check patch available and it will get back on the radar. Is derbyall clean as well l? I am not seeing the original path anymore. For future reference, I think it is good in general to keep the old ones there to see what's changed. Thank you so much for all your work on Derby!

          Show
          Kathey Marsden added a comment - - edited I am sorry for the delay in responding. If you think the new patch is read for commit can you check patch available and it will get back on the radar. Is derbyall clean as well l? I am not seeing the original path anymore. For future reference, I think it is good in general to keep the old ones there to see what's changed. Thank you so much for all your work on Derby!
          Dave Brosius made changes -
          Issue & fix info [Patch Available]
          Hide
          Lily Wei added a comment -

          Thanks Dave and Kathey. The patch looks good to me. I did not see test failure on ToolsScripts. Suites.all passed on trunk with the patch. I am running derbyall now. If there is no objection, I will check-in the patch shortly.

          Show
          Lily Wei added a comment - Thanks Dave and Kathey. The patch looks good to me. I did not see test failure on ToolsScripts. Suites.all passed on trunk with the patch. I am running derbyall now. If there is no objection, I will check-in the patch shortly.
          Rick Hillegas made changes -
          Fix Version/s 10.8.1.3 [ 12316378 ]
          Fix Version/s 10.8.1.2 [ 12316362 ]
          Hide
          Lily Wei added a comment -

          I've committed with revision 1098040. Thanks, Dave.

          Show
          Lily Wei added a comment - I've committed with revision 1098040. Thanks, Dave.
          Lily Wei made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Assignee Dave Brosius [ dbrosius@apache.org ]
          Resolution Fixed [ 1 ]
          Rick Hillegas made changes -
          Fix Version/s 10.8.1.4 [ 12316500 ]
          Fix Version/s 10.8.1.3 [ 12316378 ]
          Knut Anders Hatlen made changes -
          Fix Version/s 10.8.1.5 [ 12316676 ]
          Fix Version/s 10.8.1.4 [ 12316500 ]
          Hide
          Myrna van Lunteren added a comment -

          Correcting fix version; this is not in 10.8 branch, but eligible for backport (even though marked improvement).

          Show
          Myrna van Lunteren added a comment - Correcting fix version; this is not in 10.8 branch, but eligible for backport (even though marked improvement).
          Myrna van Lunteren made changes -
          Fix Version/s 10.9.0.0 [ 12316344 ]
          Fix Version/s 10.8.1.5 [ 12316676 ]
          Hide
          Kathey Marsden added a comment -

          Changing this to a bug. Looking at the change, I wonder if it is ok to completely throw away the errors closing statements. It would be nice to see those included with a setNextException() so all errors get reported.

          Show
          Kathey Marsden added a comment - Changing this to a bug. Looking at the change, I wonder if it is ok to completely throw away the errors closing statements. It would be nice to see those included with a setNextException() so all errors get reported.
          Kathey Marsden made changes -
          Issue Type Improvement [ 4 ] Bug [ 1 ]
          Hide
          Knut Anders Hatlen added a comment -

          [bulk update] Close all resolved issues that haven't been updated for more than one year.

          Show
          Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.
          Knut Anders Hatlen made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Gavin made changes -
          Workflow jira [ 12609063 ] Default workflow, editable Closed status [ 12802718 ]
          Hide
          Kathey Marsden added a comment -

          assign tempoarily for 10.8 backport

          Show
          Kathey Marsden added a comment - assign tempoarily for 10.8 backport
          Kathey Marsden made changes -
          Assignee Dave Brosius [ dbrosius@apache.org ] Kathey Marsden [ kmarsden ]
          Kathey Marsden made changes -
          Link This issue relates to DERBY-6289 [ DERBY-6289 ]
          Hide
          ASF subversion and git services added a comment -

          Commit 1501867 from Kathey Marsden
          [ https://svn.apache.org/r1501867 ]

          DERBY-5163 [patch] fix up sql cleanup handling

          Contributed by Dave Brosius.
          Merged 1098040 from trunk to 10.8

          Show
          ASF subversion and git services added a comment - Commit 1501867 from Kathey Marsden [ https://svn.apache.org/r1501867 ] DERBY-5163 [patch] fix up sql cleanup handling Contributed by Dave Brosius. Merged 1098040 from trunk to 10.8
          Hide
          Kathey Marsden added a comment -

          reassigning. Backport complete

          Show
          Kathey Marsden added a comment - reassigning. Backport complete
          Kathey Marsden made changes -
          Assignee Kathey Marsden [ kmarsden ] Dave Brosius [ dbrosius@apache.org ]
          Fix Version/s 10.8.3.1 [ 12323475 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          30d 23h 2m 1 Lily Wei 30/Apr/11 04:59
          Resolved Resolved Closed Closed
          779d 5h 19m 1 Knut Anders Hatlen 17/Jun/13 10:19

            People

            • Assignee:
              Dave Brosius
              Reporter:
              Dave Brosius
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development