Derby
  1. Derby
  2. DERBY-3853

Behaviour of setTypeMap() differs between embedded and client

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.5.1.1
    • Fix Version/s: 10.6.1.0
    • Component/s: JDBC
    • Labels:
      None
    • Issue & fix info:
      Newcomer
    • Bug behavior facts:
      Embedded/Client difference

      Description

      On the embedded driver, Connection.setTypeMap() behaves like this (when the connection is not closed):

      • if the map argument is null, throw an SQLException with SQLState XJ081
      • if the map is not null and not empty, throw an SQLException with SQLState 0A000
      • if the map is not null and empty, do nothing

      The behaviour on the client driver is this:

      • always throw an SQLException with SQLState 0A000

      We should try to make the two drivers behave the same way when setTypeMap() is called. (This would also allow us to simplify some of the tests in J2EEDataSourceTest).

      1. releaseNote.html
        4 kB
        Knut Anders Hatlen
      2. DERBY-3853-2.stat
        0.2 kB
        Yun Lee
      3. DERBY-3853-2.patch
        15 kB
        Yun Lee
      4. DERBY-3853-1.stat
        0.2 kB
        Yun Lee
      5. DERBY-3853-1.patch
        3 kB
        Yun Lee

        Issue Links

          Activity

          Hide
          Yun Lee added a comment -

          I have changed the EmbedConnection.setTypeMap() to act as the same way of Connection. They all throw a "0A000" Exc\eption now. The new change aslo performs in the J2EEDataSourceTest.java.

          Please check it. Thanks!

          Show
          Yun Lee added a comment - I have changed the EmbedConnection.setTypeMap() to act as the same way of Connection. They all throw a "0A000" Exc\eption now. The new change aslo performs in the J2EEDataSourceTest.java. Please check it. Thanks!
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for the patch. With that solution some of the calls to setTypeMap() that used to succeed on the embedded driver will start throwing exceptions. This may break existing applications, so I think it's probably safer to make the changes on the client driver instead.

          Some minor comments about the test changes:

          • I don't think the calls to sqle.getNextException() are necessary when invoking assertSQLState(), since assertSQLState() is supposed to follow the exception chain until it finds a match
          • Code like the example below should have a call to fail() at the end of the try block in order to detect if the code didn't throw any exception:

          + try

          { + cs.execute(); + }

          catch (SQLException sqle)

          { + assertSQLState("0A000", sqle.getNextException()); + }
          Show
          Knut Anders Hatlen added a comment - Thanks for the patch. With that solution some of the calls to setTypeMap() that used to succeed on the embedded driver will start throwing exceptions. This may break existing applications, so I think it's probably safer to make the changes on the client driver instead. Some minor comments about the test changes: I don't think the calls to sqle.getNextException() are necessary when invoking assertSQLState(), since assertSQLState() is supposed to follow the exception chain until it finds a match Code like the example below should have a call to fail() at the end of the try block in order to detect if the code didn't throw any exception: + try { + cs.execute(); + } catch (SQLException sqle) { + assertSQLState("0A000", sqle.getNextException()); + }
          Hide
          Yun Lee added a comment -

          Knut, thanks for your comments. I will turn back on weekend to provide new patch adopting your advice,

          Show
          Yun Lee added a comment - Knut, thanks for your comments. I will turn back on weekend to provide new patch adopting your advice,
          Hide
          Yun Lee added a comment -

          Knut, I have provided a new patch with changes:

          1.) make the changes on the client driver instead of on EmbedConnection;
          2.) remove sqle.getNextException() when invoking assertSQLState();
          3.) add fail() at the end of the try block in many places.

          However, I think it's the same safe to make change on EmbedConneciton with on client driver, as whatever, the test data ----EmptyMapValue, NullMapValue and MapMapValue ---- for testAllDataSource() has changed, .

          Wish for your check!

          Also, I'm a bit strange to have found the source code of Derby doesn't follow a uniform style, such as '=' accompanied by space characters, and the indent for curly braces. Aren't they?

          Yun

          Show
          Yun Lee added a comment - Knut, I have provided a new patch with changes: 1.) make the changes on the client driver instead of on EmbedConnection; 2.) remove sqle.getNextException() when invoking assertSQLState(); 3.) add fail() at the end of the try block in many places. However, I think it's the same safe to make change on EmbedConneciton with on client driver, as whatever, the test data ----EmptyMapValue, NullMapValue and MapMapValue ---- for testAllDataSource() has changed, . Wish for your check! Also, I'm a bit strange to have found the source code of Derby doesn't follow a uniform style, such as '=' accompanied by space characters, and the indent for curly braces. Aren't they? Yun
          Hide
          Knut Anders Hatlen added a comment -

          Hi Yun,

          Thanks for the new patch! It looks good to me. I'll run some tests and commit it if I don't find any problems.

          The reason why I felt it was safer to make the changes on the client driver, was that the changes there would be that things that failed would start working or that they would fail with another error code. If we had made the change on the embedded driver, things that used to work would stop working, which is worse.

          But as you said, there are changes on the client side too, so we may consider writing a release note warning users that the error code has changed. Though I'm not sure if we have bothered release noting changes from "0A000 - not implemented" before.

          You're right about the inconsistent coding style. There was a vote once where it was decided that the Java coding conventions[1] (with some amendments that I don't remember right now) was the recommended style for new code. It was not decided that the entire code base should be reformatted, so we still have to live with different styles.

          [1] http://java.sun.com/docs/codeconv/html/CodeConvTOC.doc.html

          Show
          Knut Anders Hatlen added a comment - Hi Yun, Thanks for the new patch! It looks good to me. I'll run some tests and commit it if I don't find any problems. The reason why I felt it was safer to make the changes on the client driver, was that the changes there would be that things that failed would start working or that they would fail with another error code. If we had made the change on the embedded driver, things that used to work would stop working, which is worse. But as you said, there are changes on the client side too, so we may consider writing a release note warning users that the error code has changed. Though I'm not sure if we have bothered release noting changes from "0A000 - not implemented" before. You're right about the inconsistent coding style. There was a vote once where it was decided that the Java coding conventions [1] (with some amendments that I don't remember right now) was the recommended style for new code. It was not decided that the entire code base should be reformatted, so we still have to live with different styles. [1] http://java.sun.com/docs/codeconv/html/CodeConvTOC.doc.html
          Hide
          Yun Lee added a comment -

          Knut, I'm happy to hear it will be commited soon! Thanks!

          By what you explained so clearly, I've got why it's better to make changes on client side now.

          I have browsed the coding convention doc, and will follow it in my work on Derby.

          Yun

          Show
          Yun Lee added a comment - Knut, I'm happy to hear it will be commited soon! Thanks! By what you explained so clearly, I've got why it's better to make changes on client side now. I have browsed the coding convention doc, and will follow it in my work on Derby. Yun
          Hide
          Knut Anders Hatlen added a comment -

          Committed revision 764217.
          I wasn't planning to backport this fix to the 10.5 branch.

          Checking "Release Note Needed" since the fix changes an error code.

          Show
          Knut Anders Hatlen added a comment - Committed revision 764217. I wasn't planning to backport this fix to the 10.5 branch. Checking "Release Note Needed" since the fix changes an error code.
          Hide
          Knut Anders Hatlen added a comment -

          Here's an attempt on a release note for this issue.

          Show
          Knut Anders Hatlen added a comment - Here's an attempt on a release note for this issue.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development