On Fri, Nov 15, 2013 at 09:14:58AM -0800, David E. Wheeler wrote:
> On Nov 15, 2013, at 3:12 AM, Tim Bunce <tim.bu...@pobox.com> wrote:
>
> > You'd need to write the callback code in the way you'd naturally write
> > it when not using RaiseError. Which typically means something like:
> >
> > $dbh->do('SET search_path = ?', undef, 'foo')
> > or return;
>
> That will prevent any further code from executing, but does not give me a
> chance to handle the error.
When the method call (which fired the callback) returns, the error
recorded on the handle will trigger RaiseError etc.
> I *always* use RaiseError => 1 (or HandleError). Never ever check return
> values.
Ah, your use of HandleError adds an extra perspective on this.
> > Probably. There are three main down-sides to consider:
> > 1. compatibility - the risk of breaking some existing callback code
>
> Seems unlikely, I think, since it’s not documented that it’s not the
> user’s handle that is passed to callbacks. I think I have been using
> callbacks longer than anyone and never noticed. Frankly I’m amazed at
> how few people know they’re there (I regularly get requests to add
> them to DBIx::Connector).
I agree (on all points :).
> > 2. performance - the cost of getting the outer handle
> > 3. pure-perl compatibility
> >
> > There is an undocumented DBI::_handles function that returns the inner
> > and outer handles for a given (inner or outer) handle. Perl code within
> > DBI.pm, such as the connect_cached callback logic, would need to call
> > that (or something like it) to get the outer handle to pass to the callback.
>
> Yeah. That’s not a ton over overhead, is it? Is an additional method
> call really likely to have much of a performance impact?
Probably not. I think connect_cached is the only use of Callbacks in
DBI.pm (not DBI.xs), and the very small added cost would only be paid by
those using Callbacks anyway.
> > The DBI::_handles implementation in lib/DBI/PurePerl.pm doesn't support
> > getting the outer handle from an inner one. That's probably fixable by
> > adding a weakref.
> >
> > I don't have a strong opinion on this.
>
> I am strongly in favor of providing a consistent interface to users.
> Currently, error-handling in callbacks is inconsistent when using
> RaiseError or HandleError.
I agree.
> I can carve out a little time today to write some test cases if it’ll help.
No need, I've just hacked t/70callbacks.t to include those tests :)
Tim.
