Derby
  1. Derby
  2. DERBY-2720

remove dead code associated with unsupported National Char implementation

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.3.3.1, 10.9.1.0
    • Component/s: SQL
    • Labels:
      None

      Description

      Derby still has some untested, unused code relating to a non-standard implementation of a Nationa Char type. The current code can be removed.
      I believe the interesting functionality associated with this is now provided by DERBY-1478 (territory based collation) . If Derby ever implements a
      National Char type it should do so differently than the existing code, collation should not be tied to the National Char type.

      I believe a future National char type might have to maintain a separate type id for compatibility with jdbc interface, but actual implmentation should be
      the same code as the char types. Collating of the the national char type should be supported in exactly same way as regular char types.

      If anyone is really intested in the national char code, it's history will always be available in svn, and a consistent version is available by looking at 10.0, 10.1,
      and 10.2 codelines. I would propose any removal of code only take place in trunk and not be backported to a released codeline.

        Activity

        Hide
        Mamta A. Satoor added a comment -

        I have removed the disabled national character code from trunk. The changes went in as part of revision 612525 into trunk and the tests ran fine with Sun's jdk14 on Windows XP machine. I will port these changes into 10.3 too.

        Show
        Mamta A. Satoor added a comment - I have removed the disabled national character code from trunk. The changes went in as part of revision 612525 into trunk and the tests ran fine with Sun's jdk14 on Windows XP machine. I will port these changes into 10.3 too.
        Hide
        Mamta A. Satoor added a comment -

        Merged changes from trunk(612525) into 10.3 codeline(revision 612576).

        Show
        Mamta A. Satoor added a comment - Merged changes from trunk(612525) into 10.3 codeline(revision 612576).
        Hide
        Mamta A. Satoor added a comment -

        The checkin above caused test failure in SetObjectUnsupportedTest which runs under JDK1.6. Unfortunately, I ran my tests only with JDK1.4 and hence I didn't catch the problem before checkin. I just fixed the problem with revision 612865 in trunk. Will work on migrating it to 10.3 The commit comments for trunk were as follows

        While working on DERBY-2720, I removed national datatypes as one of the unsupported data types(revision 612525) and that caused the test failure for SetObjectUnsupportedTest since we stopped recognizing national dataypes as one of the unsupported datatypes. The fix required changes in only one class which I had modified incorrectly in revision 612525. This test failure was not caught earlier by my local runs of the test because I had used JDK14 which has JDBC3 support and this paritcular test requires JDBC4 support.

        Show
        Mamta A. Satoor added a comment - The checkin above caused test failure in SetObjectUnsupportedTest which runs under JDK1.6. Unfortunately, I ran my tests only with JDK1.4 and hence I didn't catch the problem before checkin. I just fixed the problem with revision 612865 in trunk. Will work on migrating it to 10.3 The commit comments for trunk were as follows While working on DERBY-2720 , I removed national datatypes as one of the unsupported data types(revision 612525) and that caused the test failure for SetObjectUnsupportedTest since we stopped recognizing national dataypes as one of the unsupported datatypes. The fix required changes in only one class which I had modified incorrectly in revision 612525. This test failure was not caught earlier by my local runs of the test because I had used JDK14 which has JDBC3 support and this paritcular test requires JDBC4 support.
        Hide
        Mamta A. Satoor added a comment -

        Merged the trunk revision 612865 into 10.3 codeline (revision 612872)

        Show
        Mamta A. Satoor added a comment - Merged the trunk revision 612865 into 10.3 codeline (revision 612872)
        Hide
        Daniel John Debrunner added a comment -

        Some of the code that the national types (incorrectly) used to convert between datetime values and national characters has not been removed.

        E.g. the SQLChar.get

        {Date,Time.Timestamp}

        Format methods are not used (I think).
        Removing these methods may show other methods not used and may progress to the date time methods in LocaleFinder being removed.

        I'm testing removing the getXXXFormat() methods.

        Show
        Daniel John Debrunner added a comment - Some of the code that the national types (incorrectly) used to convert between datetime values and national characters has not been removed. E.g. the SQLChar.get {Date,Time.Timestamp} Format methods are not used (I think). Removing these methods may show other methods not used and may progress to the date time methods in LocaleFinder being removed. I'm testing removing the getXXXFormat() methods.
        Hide
        Kristian Waagan added a comment -

        Patch 1a removes the methods mentioned by Dan.

        suites.All and derbyall ran without failures.
        I plan to commit this shortly.

        Show
        Kristian Waagan added a comment - Patch 1a removes the methods mentioned by Dan. suites.All and derbyall ran without failures. I plan to commit this shortly.
        Hide
        Kristian Waagan added a comment -

        Committed patch 1a to trunk with revision 1172535.

        Show
        Kristian Waagan added a comment - Committed patch 1a to trunk with revision 1172535.
        Hide
        Kristian Waagan added a comment -

        Attaching patch 2a, which removed the method DataType.getNationalString and its implementations.

        Regression tests ran without failures.
        Patch ready for review.

        I plan to commit this patch shortly.

        Show
        Kristian Waagan added a comment - Attaching patch 2a, which removed the method DataType.getNationalString and its implementations. Regression tests ran without failures. Patch ready for review. I plan to commit this patch shortly.
        Hide
        Knut Anders Hatlen added a comment -

        Looks good to me. +1

        Show
        Knut Anders Hatlen added a comment - Looks good to me. +1
        Hide
        Kristian Waagan added a comment -

        Thanks, Knut Anders.

        Committed patch 2a to trunk with revision 1181679.

        I don't know if it is a goal in itself to remove LocaleFinder, but if so a separate JIRA should be created for that task.
        There may be more dead code left, in that case I also suggest creating a new JIRA.

        Show
        Kristian Waagan added a comment - Thanks, Knut Anders. Committed patch 2a to trunk with revision 1181679. I don't know if it is a goal in itself to remove LocaleFinder, but if so a separate JIRA should be created for that task. There may be more dead code left, in that case I also suggest creating a new JIRA.
        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.

          People

          • Assignee:
            Mamta A. Satoor
            Reporter:
            Mike Matrigali
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development