commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <>
Subject Re: [dbcp] Changes in R734470, 734481 to replace SQLNestedException
Date Wed, 21 Jan 2009 21:16:12 GMT
Phil Steitz wrote:
> sebb wrote:
>> On 20/01/2009, Phil Steitz <> wrote:
>>> I would like to ask that these changes be reverted.   Deprecated
>>> classes and
>>> client code breakage that depends on them can only happen in major (x.0)
>>> releases.  Unless I am missing something subtle, these changes will
>>> break
>>> client code that specializes the cast on caught exceptions and tries to
>>> unpack the nested root cause exception.
>> Throwable.getCause() was added in 1.4, and DBCP now targets 1.5+
>> (according to the Maven build files) so there won't be a problem
>> calling getCause(), however the cast to SQLNestedException would fail.
>>> Since this is a .y release, we
>>> should not make this change at this point.
>> OK, I was a bit hasty (though I did ask first, I did not wait long
>> enough).
> Sorry I took so long to review.
>> Rather than remove the new code, how about changing:
>> throw (SQLException) new SQLException(message).initCause(e);
>> into:
>> throw (SQLException) new SQLNestedException(message).setCause(e);
>> The SQLNestedException class would need to be extended to add a
>> (String) constructor and a setCause() method which copies the relevant
>> code from the existing constructor.
>> That is easy to do now and easy to undo when the time comes to remove
>> the deprecated class. I think it maintains full compatibility as well.
>> [If overriding Throwable.initCause() is allowed, then the change could
>> be simplified slightly]
>> WDYT?
> Honestly, I think the best solution is to revert the change.  When, if
> ever, DBCP sees a 2.0, we can clean remove SQLNestedException and clean
> up systematically.  I don't see the risk of other unanticipated impacts
> as being worth the simplification in cleanup later.  We would also have
> to add test cases.  So my vote is for less work  and breakage risk now,
> which means revert the change.



To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message