Derby
  1. Derby
  2. DERBY-3371

Strange (and untested) code fragment in RAMTransaction.addColumnToConglomerate()

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.4.1.3
    • Fix Version/s: 10.10.1.1
    • Component/s: Store
    • Labels:
      None

      Description

      RAMTransaction.addColumnToConglomerate() contains this piece of code:

      // remove old entry in the Conglomerate directory, and add new one
      if (tempCongloms != null)
      tempCongloms.remove(new Long(conglomId));
      tempCongloms.put(new Long(conglomId), conglom);

      1. According to the code coverage report (http://people.apache.org/~fuzzylogic/codecoverage/529822/_files/3fc.html#5) these lines are not tested. If possible, a test that covers them should be added to the regression suite.

      2. The null check looks either unnecessary (seems to be the case after a brief inspection of the code), or incomplete since the last line will throw a NullPointerException regardless of the check if tempCongloms is null.

      3. The call to remove() before put() is redundant, since HashMap.put() will remove the old mapping implicitly.

      4. It seems to me that the object that is put into the HashMap always is the same as the one that is removed, so perhaps all these lines could be deleted.

      1. test.diff
        2 kB
        Knut Anders Hatlen
      2. remove.diff
        1.0 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Knut Anders Hatlen created issue -
          Hide
          Knut Anders Hatlen added a comment -

          The attached patch makes T_AccessFactory.alterTable() run tests on both temporary and non-temporary conglomerates. This change ensures that the above mentioned code fragment is executed in the tests.

          Show
          Knut Anders Hatlen added a comment - The attached patch makes T_AccessFactory.alterTable() run tests on both temporary and non-temporary conglomerates. This change ensures that the above mentioned code fragment is executed in the tests.
          Knut Anders Hatlen made changes -
          Field Original Value New Value
          Attachment test.diff [ 12374534 ]
          Hide
          Knut Anders Hatlen added a comment -

          Committed the test patch with revision 618299.

          Show
          Knut Anders Hatlen added a comment - Committed the test patch with revision 618299.
          Knut Anders Hatlen made changes -
          Link This issue is duplicated by DERBY-5285 [ DERBY-5285 ]
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a patch that removes the unneeded lines. Regression tests ran cleanly with the patch.

          Show
          Knut Anders Hatlen added a comment - Attaching a patch that removes the unneeded lines. Regression tests ran cleanly with the patch.
          Knut Anders Hatlen made changes -
          Attachment remove.diff [ 12528738 ]
          Knut Anders Hatlen made changes -
          Assignee Knut Anders Hatlen [ knutanders ]
          Issue & fix info Patch Available [ 10102 ]
          Knut Anders Hatlen made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Knut Anders Hatlen added a comment -

          Committed revision 1342566.

          Show
          Knut Anders Hatlen added a comment - Committed revision 1342566.
          Knut Anders Hatlen made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Issue & fix info Patch Available [ 10102 ]
          Fix Version/s 10.10.0.0 [ 12321550 ]
          Resolution Fixed [ 1 ]
          Knut Anders Hatlen made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Gavin made changes -
          Workflow jira [ 12422454 ] Default workflow, editable Closed status [ 12797033 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open In Progress In Progress
          1575d 16h 46m 1 Knut Anders Hatlen 25/May/12 11:44
          In Progress In Progress Resolved Resolved
          1m 6s 1 Knut Anders Hatlen 25/May/12 11:45
          Resolved Resolved Closed Closed
          60d 6h 42m 1 Knut Anders Hatlen 24/Jul/12 18:28

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development