Derby
  1. Derby
  2. DERBY-3450 Reduce module dependencies in Derby
  3. DERBY-3453

The PublicAPI class should belong to org.apache.derby.iapi.jdbc as it handles creation of SQLException

    Details

    • Type: Sub-task Sub-task
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: JDBC
    • Urgency:
      Normal

      Description

      The PublicAPI class should belong to org.apache.derby.iapi.jdbc as it handles creation of SQLException.
      The classes EmbedSQLException and EmbedSQLWarning are better placed in org.apache.derby.iapi.jdbc. These classes are referenced outside the jdbc module.

        Issue Links

          Activity

          Mamta A. Satoor made changes -
          Labels derby_triage10_11
          Urgency Normal [ 10052 ]
          Gavin made changes -
          Workflow jira [ 12424259 ] Default workflow, editable Closed status [ 12799262 ]
          Gavin made changes -
          Link This issue depends upon DERBY-3451 [ DERBY-3451 ]
          Gavin made changes -
          Link This issue depends on DERBY-3451 [ DERBY-3451 ]
          Kathey Marsden made changes -
          Assignee Dibyendu Majumdar [ dibyendumajumdar ]
          Hide
          Kathey Marsden added a comment -

          Unassigning this issue as there has been no activity for over a year. Dibyendu, please reassign yourself if you would like to continue work on this issue.

          Show
          Kathey Marsden added a comment - Unassigning this issue as there has been no activity for over a year. Dibyendu, please reassign yourself if you would like to continue work on this issue.
          Kathey Marsden made changes -
          Component/s JDBC [ 11407 ]
          Hide
          Dibyendu Majumdar added a comment -

          I am looking at removing EmbedSQLException altogether as suggested by Dan, in favour of SQLException.
          It seems that this is feasible.
          EmbedSQLException extends SQLException by adding some of the attributes of StandardException - in particular the messageId, and the arguments array.
          This will be handled in a uniform manner by always setting a StandardException object as the cause of the SQLException.

          As a result of this change, I expect that the classes PublicAPI, o.a.d.impl.jdbc.Util, and EmbedSQLException will be removed.
          The SQLExceptionFactory classes are likely to become generic and therefore will get moved to o.a.d.iapi.error.

          This change will simplify the exception classes and remove some of the unnecessary dependencies between the modules.

          Show
          Dibyendu Majumdar added a comment - I am looking at removing EmbedSQLException altogether as suggested by Dan, in favour of SQLException. It seems that this is feasible. EmbedSQLException extends SQLException by adding some of the attributes of StandardException - in particular the messageId, and the arguments array. This will be handled in a uniform manner by always setting a StandardException object as the cause of the SQLException. As a result of this change, I expect that the classes PublicAPI, o.a.d.impl.jdbc.Util, and EmbedSQLException will be removed. The SQLExceptionFactory classes are likely to become generic and therefore will get moved to o.a.d.iapi.error. This change will simplify the exception classes and remove some of the unnecessary dependencies between the modules.
          Dibyendu Majumdar made changes -
          Attachment DERBY-3453_patch_rev630512.txt [ 12376327 ]
          Dibyendu Majumdar made changes -
          Derby Info [Patch Available]
          Hide
          Dibyendu Majumdar added a comment -

          Unsetting patch available flag as this patch will be reworked to remove changes to EmbedSQLWarning.

          Show
          Dibyendu Majumdar added a comment - Unsetting patch available flag as this patch will be reworked to remove changes to EmbedSQLWarning.
          Hide
          Dibyendu Majumdar added a comment -

          Okay, I will separate the EmbedSQLWarning into a separate cleanup. If the only dependency is on a standard java library class, then o.a.d.iapi.error is a good package for the renamed class.

          I think that the whole business of EmbedSQLException, EmbedSQLWarning, PublicAPI, and o.a.d.impl.jdbc.Util needs to be looked at. But, rather than making big revisions, I wanted to make small incremental changes. I am sure that as I work my way up the layers, more re-factoring will be necessary.

          Would the proposed changes to PublicAPI and EmbedSQLException be acceptable as interim changes?

          Show
          Dibyendu Majumdar added a comment - Okay, I will separate the EmbedSQLWarning into a separate cleanup. If the only dependency is on a standard java library class, then o.a.d.iapi.error is a good package for the renamed class. I think that the whole business of EmbedSQLException, EmbedSQLWarning, PublicAPI, and o.a.d.impl.jdbc.Util needs to be looked at. But, rather than making big revisions, I wanted to make small incremental changes. I am sure that as I work my way up the layers, more re-factoring will be necessary. Would the proposed changes to PublicAPI and EmbedSQLException be acceptable as interim changes?
          Daniel John Debrunner made changes -
          Link This issue is related to DERBY-3461 [ DERBY-3461 ]
          Hide
          Daniel John Debrunner added a comment -

          Will org.apache.derby.iapi.jdbc be the correct location for EmbedSQLWarning? It's used by the language code, but I understand a goal of yours is to not have the language code depend on the jdbcapi layer.

          EmbedSQLWarning is really a factory class to return SQLWarning objects, and it should return plain java.sql.SQLWarning objects rather than a Derby specific implementation. Assume a re-name to SQLWarningFactory, then it would depend on only on the message service. Since this is really related to error handling should SQLWarningFactory belong in org.apache.derby.iapi.error?

          I think the EmbedSQLWarning cleanup should be a separate issue.

          PublicAPI may also benefit from any improvement not to use EmbedSQLException, thus it may be worth addressing that first.

          Also the replication code is using PublicAPI but seems to be changing its approach, thus it may be better to hold off this patch until that code has been updated.

          Show
          Daniel John Debrunner added a comment - Will org.apache.derby.iapi.jdbc be the correct location for EmbedSQLWarning? It's used by the language code, but I understand a goal of yours is to not have the language code depend on the jdbcapi layer. EmbedSQLWarning is really a factory class to return SQLWarning objects, and it should return plain java.sql.SQLWarning objects rather than a Derby specific implementation. Assume a re-name to SQLWarningFactory, then it would depend on only on the message service. Since this is really related to error handling should SQLWarningFactory belong in org.apache.derby.iapi.error? I think the EmbedSQLWarning cleanup should be a separate issue. PublicAPI may also benefit from any improvement not to use EmbedSQLException, thus it may be worth addressing that first. Also the replication code is using PublicAPI but seems to be changing its approach, thus it may be better to hold off this patch until that code has been updated.
          Dibyendu Majumdar made changes -
          Attachment DERBY-3453_patch_rev630512.txt [ 12376327 ]
          Hide
          Dibyendu Majumdar added a comment -

          Please see DERBY-3451 for test results and how to apply this patch.
          This patch should be applied after DERBY-3452.

          Show
          Dibyendu Majumdar added a comment - Please see DERBY-3451 for test results and how to apply this patch. This patch should be applied after DERBY-3452 .
          Dibyendu Majumdar made changes -
          Attachment DERBY-3453_patch_1.txt [ 12376286 ]
          Hide
          Dibyendu Majumdar added a comment -

          For test results, see DERBY-3451.

          Show
          Dibyendu Majumdar added a comment - For test results, see DERBY-3451 .
          Dibyendu Majumdar made changes -
          Derby Info [Patch Available]
          Dibyendu Majumdar made changes -
          Attachment DERBY-3453_patch_1.txt [ 12376286 ]
          Hide
          Dibyendu Majumdar added a comment -

          The PublicAPI class has been moved to org.apache.derby.iapi.jdbc.
          The EmbedSQLException and EmbedSQLWarning classes have been moved out of org.apache.derby.impl.jdbc to org.apache.derby.iapi.jdbc.

          Show
          Dibyendu Majumdar added a comment - The PublicAPI class has been moved to org.apache.derby.iapi.jdbc. The EmbedSQLException and EmbedSQLWarning classes have been moved out of org.apache.derby.impl.jdbc to org.apache.derby.iapi.jdbc.
          Hide
          Dibyendu Majumdar added a comment -

          Apply the patch for DERBY-3451 before applying patches for this task.

          Show
          Dibyendu Majumdar added a comment - Apply the patch for DERBY-3451 before applying patches for this task.
          Dibyendu Majumdar made changes -
          Field Original Value New Value
          Link This issue depends on DERBY-3451 [ DERBY-3451 ]
          Dibyendu Majumdar created issue -

            People

            • Assignee:
              Unassigned
              Reporter:
              Dibyendu Majumdar
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:

                Development