Derby
  1. Derby
  2. DERBY-3600

Change replication methods in org.apache.derby.iapi.db.Database to throw StandardException instead of SQLException

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: 10.6.1.0
    • Fix Version/s: None
    • Component/s: Replication
    • Labels:
      None
    • Issue & fix info:
      Newcomer

      Issue Links

        Activity

        Hide
        Tiago R. Espinha added a comment -

        Triaged for 10.5.2.

        • Checked Newcomer.
        • Changed affects versions to 10.6.0.0 and type to improvement.
        • Reduced priority.
        • Unassigned the issue as it seems abandoned.
        Show
        Tiago R. Espinha added a comment - Triaged for 10.5.2. Checked Newcomer. Changed affects versions to 10.6.0.0 and type to improvement. Reduced priority. Unassigned the issue as it seems abandoned.
        Hide
        Danoja Dias added a comment -

        I added the patch to throw StandardException instead of SQLException.

        Show
        Danoja Dias added a comment - I added the patch to throw StandardException instead of SQLException.
        Hide
        Bryan Pendleton added a comment -

        Your patch seems straightforward.

        What sort of testing have you done? Have you been able to detect
        any changes in behavior due to this change?

        I'm not totally certain what sort of behavior changes we ought to
        expect. This is a rather old job, and unfortunately it doesn't
        have any clear description of *why* it would be better to
        throw StandardException rather than SQLException, so we may
        have to do some experimentation to try to figure out what we
        should be looking for as the results of making this change.

        Show
        Bryan Pendleton added a comment - Your patch seems straightforward. What sort of testing have you done? Have you been able to detect any changes in behavior due to this change? I'm not totally certain what sort of behavior changes we ought to expect. This is a rather old job, and unfortunately it doesn't have any clear description of * why * it would be better to throw StandardException rather than SQLException, so we may have to do some experimentation to try to figure out what we should be looking for as the results of making this change.
        Hide
        Danoja Dias added a comment -

        What sort of testing have you done? Have you been able to detect
        any changes in behavior due to this change?

        I ran all tests and they were clean. I didn't find any changes due to this.

        I also had the problem that why we should do this.

        Show
        Danoja Dias added a comment - What sort of testing have you done? Have you been able to detect any changes in behavior due to this change? I ran all tests and they were clean. I didn't find any changes due to this. I also had the problem that why we should do this.
        Hide
        Bryan Pendleton added a comment -

        Perhaps you could look through the archives of the derby-dev
        mailing list for the week or two right before 07-apr-2008,
        when this job was logged – sometimes, there is a discussion
        on the developers list which gives insight about why a particular
        job was logged.

        Show
        Bryan Pendleton added a comment - Perhaps you could look through the archives of the derby-dev mailing list for the week or two right before 07-apr-2008, when this job was logged – sometimes, there is a discussion on the developers list which gives insight about why a particular job was logged.
        Hide
        Danoja Dias added a comment -

        This is a part of DERBY-3455. Linking DERBY-3455 to this issue.

        Show
        Danoja Dias added a comment - This is a part of DERBY-3455 . Linking DERBY-3455 to this issue.
        Hide
        Danoja Dias added a comment -

        I checked mail archives and found that this is a part of DERBY-3455

        Show
        Danoja Dias added a comment - I checked mail archives and found that this is a part of DERBY-3455
        Hide
        Bryan Pendleton added a comment -

        Ah, so this task was a cleanup activity that was observed
        during code review, but was not completed (yet).

        It's interesting that it has been 8 years now, and there have
        been no symptoms noticed from this particular choice of
        exception signature.

        It appears that DERBY-3428 is also involved, as that was the initial project where the code review occurred.

        Let's look through those issues in more detail, and look at the code in question with fresh eyes, and see if we can understand
        more about what issues the reviewers were concerned with.

        Show
        Bryan Pendleton added a comment - Ah, so this task was a cleanup activity that was observed during code review, but was not completed (yet). It's interesting that it has been 8 years now, and there have been no symptoms noticed from this particular choice of exception signature. It appears that DERBY-3428 is also involved, as that was the initial project where the code review occurred. Let's look through those issues in more detail, and look at the code in question with fresh eyes, and see if we can understand more about what issues the reviewers were concerned with.
        Hide
        Bryan Pendleton added a comment -

        After studying the related JIRAs, and looking through the derby-dev archives
        for January/February 2008, my feeling is that we should close this issue as "wont fix".

        This issue is 8 1/2 years old, and my instinct is that changing the exception
        signatures at this point brings more risk than reward.

        I think the most relevant comment is this one, from DERBY-3455: https://issues.apache.org/jira/browse/DERBY-3455?focusedCommentId=12572846&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-12572846

        I hope there will be a follow-up patch that unwraps the SQLExcpetions into
        StandardException for these methods.

        The use of the term "unwraps" makes me think that the primary concern was that if one of
        these exceptions was raised, it would be wrapped, unnecessarily, in multiple intermediate
        "wrapper" exceptions, which would confuse the diagnose and potentially conceal the
        root cause.

        But that concern doesn't seem to warrant modifying this code at this late date.

        Show
        Bryan Pendleton added a comment - After studying the related JIRAs, and looking through the derby-dev archives for January/February 2008, my feeling is that we should close this issue as "wont fix". This issue is 8 1/2 years old, and my instinct is that changing the exception signatures at this point brings more risk than reward. I think the most relevant comment is this one, from DERBY-3455 : https://issues.apache.org/jira/browse/DERBY-3455?focusedCommentId=12572846&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-12572846 I hope there will be a follow-up patch that unwraps the SQLExcpetions into StandardException for these methods. The use of the term "unwraps" makes me think that the primary concern was that if one of these exceptions was raised, it would be wrapped, unnecessarily, in multiple intermediate "wrapper" exceptions, which would confuse the diagnose and potentially conceal the root cause. But that concern doesn't seem to warrant modifying this code at this late date.
        Hide
        Danoja Dias added a comment -

        Then it will be a risk of doing this. We can close this issue.

        Show
        Danoja Dias added a comment - Then it will be a risk of doing this. We can close this issue.
        Hide
        Bryan Pendleton added a comment -

        After considerable study, it is our opinion that the risks of changing
        this code at this time outweigh the potential benefts as we understand them.

        Closing as "wont fix"

        Show
        Bryan Pendleton added a comment - After considerable study, it is our opinion that the risks of changing this code at this time outweigh the potential benefts as we understand them. Closing as "wont fix"

          People

          • Assignee:
            Danoja Dias
            Reporter:
            V.Narayanan
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development