Derby
  1. Derby
  2. DERBY-4313

JDBC.dropUsingDMD() may skip dropping objects

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.6.1.0
    • Fix Version/s: 10.6.1.0
    • Component/s: Test
    • Labels:
      None
    • Urgency:
      Normal

      Description

      This loop in JDBC.dropUsingDMD() looks broken:

      // Remove any statements from the list that succeeded.
      boolean didDrop = false;
      for (int i = 0; i < results.length; i++)

      { int result = results[i]; if (result == -3 /* Statement.EXECUTE_FAILED*/) hadError = true; else if (result == -2/*Statement.SUCCESS_NO_INFO*/) didDrop = true; else if (result >= 0) didDrop = true; else Assert.fail("Negative executeBatch status"); if (didDrop) ddl.set(i, null); }

      It is supposed to check the status of each individual statement executed in a batch, and clear the successful ones from an ArrayList so that only the unsuccessful ones are left.

      However, if one of the statements is reported to have failed, and one of the proceeding statements has been successful, the failed statement will be removed from the ArrayList because didDrop is true. This means that some of the failed statements are not retried later. The statements normally fail because some other object depends on the object being dropped, and they usually succeed when they are retried after the other objects have been dropped. By not retrying, some objects that were supposed to be dropped may be left in the database.

      1. dropUsingDMD.diff
        0.9 kB
        Knut Anders Hatlen

        Activity

        Hide
        Knut Anders Hatlen added a comment -

        I tested this manually by making dropUsingDMD() public and use it to drop these three tables:

        s.execute("create table a(x int)");
        s.execute("create table b(x int primary key)");
        s.execute("create table c(x int references b)");

        Using the embedded driver, all the tables were dropped after dropUsingDMD() had returned. Using the client driver, the table B was still there after dropUsingDMD().

        The difference seems to be that the embedded driver stops executing the statements in the batch after the first failure and only returns the status for the succeeding statements, whereas the client driver continues executing statements after the first failure in the batch. I'll log a separate issue for the difference between client and embedded in handling errors in a batch. Only the bug in dropUsingDMD() will be handled in this issue.

        Show
        Knut Anders Hatlen added a comment - I tested this manually by making dropUsingDMD() public and use it to drop these three tables: s.execute("create table a(x int)"); s.execute("create table b(x int primary key)"); s.execute("create table c(x int references b )"); Using the embedded driver, all the tables were dropped after dropUsingDMD() had returned. Using the client driver, the table B was still there after dropUsingDMD(). The difference seems to be that the embedded driver stops executing the statements in the batch after the first failure and only returns the status for the succeeding statements, whereas the client driver continues executing statements after the first failure in the batch. I'll log a separate issue for the difference between client and embedded in handling errors in a batch. Only the bug in dropUsingDMD() will be handled in this issue.
        Hide
        Knut Anders Hatlen added a comment -

        dropUsingDMD.diff fixes the loop by only removing the DDL statement from the ArrayList if it had succeeded, which appears to be the intent of the code judging by the comments. The three tables mentioned in my previous comment were successfully dropped when this patch was applied. I'll also run suites.All to see if it causes problems for any of the tests.

        Show
        Knut Anders Hatlen added a comment - dropUsingDMD.diff fixes the loop by only removing the DDL statement from the ArrayList if it had succeeded, which appears to be the intent of the code judging by the comments. The three tables mentioned in my previous comment were successfully dropped when this patch was applied. I'll also run suites.All to see if it causes problems for any of the tests.
        Hide
        Knut Anders Hatlen added a comment -

        suites.All ran cleanly with the patch. I haven't run derbyall since the patch only touches code used by the JUnit tests.

        Show
        Knut Anders Hatlen added a comment - suites.All ran cleanly with the patch. I haven't run derbyall since the patch only touches code used by the JUnit tests.
        Hide
        Knut Anders Hatlen added a comment -

        Committed revision 796834.

        Show
        Knut Anders Hatlen added a comment - Committed revision 796834.

          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