On Tue, Feb 01, 2005 at 09:16:49AM -0800, David Wheeler wrote:
> On Feb 1, 2005, at 2:44 AM, Tim Bunce wrote:
>
> >Depends what you mean by wrong. I don't see any 'bugs' here.
>
> No, I wouldn't call it a bug, but it was unexpected behavior (for me,
> at least).
>
> >DBI->connect and connect_cached both explicitly set any supplied
> >attributes plus some implicit defaults like PrintError=1 and
> >AutoCommit=1.
>
> Yes, I figured that. I couldn't figure out how it was doing it, though,
> looking at the code:
>
> sub connect_cached {
Try DBI->connect_cached which calls DBI->connect which does:
unless ($dbh = $drh->$connect_meth($dsn, $user, $pass, $attr)) {
...
}
...
if (%$attr) {
...
my $a;
foreach $a (qw(RaiseError PrintError AutoCommit)) { # do these first
next unless exists $attr->{$a};
$dbh->{$a} = delete $attr->{$a};
}
foreach $a (keys %$attr) {
eval { $dbh->{$a} = $attr->{$a} } or $@ && warn $@;
}
}
Note that drivers have the opportunity to apply %$attr themselves
during their own connect call and deleted those applied attributes
from %$attr so the DBI won't have any left to apply.
> >You could certainly argue that connect_cached shouldn't do that.
> >On the other hand, I'd argue that it should perhaps compare the
> >existing
> >and new values and warn if they differ.
>
> I would argue that there should be an attribute to specify whether it
> does or not (with it doing it by default for the sake of backwards
> compatibility). I implemented a method like this in my database
> library:
>
> sub _dbh { DBI->connect_cached(...) }
>
> And I have many different methods within the module that fetch the
> database handle at different points in a transaction. I think I'd
> rather not have to cache it some other place every time I start a
> transaction to be sure that I don't get it from connect_cached again
> until the transaction is committed or rolled back. I was hoping to just
> let connect_cached() cache it for me. This is especially tricky when
> I'm writing tests, where I have setup code that starts a transaction
> and teardown code that rolls it back. These, obviously, need to exist
> completely independent of the library that I'm testing.
>
> So how about a NoReset attribute or some such to be passed to
> connect_cached()?
Certainly could be done. The DBI's default DBI::_::dr::connect_cached
method would just need to do %$attr = () if ...;
> >I think the docs say somewhere that attributes shouldn't be altered
> >when using connect_cached (because you can get this kind of problem).
> >The connect_cached docs need to spell it out though.
>
> Yes, that would help, too. But a new attribute would at allow you to
> have your cake and let me eat mine, too.
Here's what I've added to the docs for now:
---snip---
Caching connections can be useful in some applications, but it can
also cause problems, such as too many connections, and so should
be used with care. In particular, avoid changing the attributes of
a database handle created via connect_cached() because it will affect
other code that may be using the same handle.
Where multiple separate parts of a program are using connect_cached()
to connect to the same database with the same (initial) attributes
it is a good idea to add a private attribute to the connect_cached()
call to effectively limit the scope of the caching. For example:
DBI->connect_cached(..., { private_foo_cachekey => "Bar", ... });
Handles returned from that connect_cached() call will only be returned
by other connect_cached() call elsewhere in the code if those other
calls also pass in the same attribute values, including the private one.
(I've used C<private_foo_cachekey> here as an example, you can use
any attribute name with a C<private_> prefix.)
Taking that one step further, you can limit a particular connect_cached()
call to return handles unique to that one place in the code by setting the
private attribute to a unique value for that place:
DBI->connect_cached(..., { private_foo_cachekey => __FILE__.__LINE__, ... });
By using a private attribute you still get connection caching for
the individual calls to connect_cached() but, by making separate
database conections for separate parts of the code, the database
handles are isolated from any attribute changes made to other handles.
---snip---
At this point I'm undecided about supporting something like a NoReset
attribute - but mainly because it would probably become the default
behaviour if I make other changes to handle creation that I'm thinking of.
Also, it may be possible to get the same effect by adding more
general callback hooks:
$dbh = DBI->connect_cached(..., {
OnConnect => sub { print "New connection" };
OnConnectCached => sub { print "Old connection"; my (...,$attr)[EMAIL
PROTECTED]; %$attr = () };
});
Still, it's too late for 1.47 now.
Tim.
