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

          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)
          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?
          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.
          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.
          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)
          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!
          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.
          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.
          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).
          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.
          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.
          Hide
          Kathey Marsden added a comment -

          assign tempoarily for 10.8 backport

          Show
          Kathey Marsden added a comment - assign tempoarily for 10.8 backport
          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

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development