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 { my $drh = shift; my ($dsn, $user, $auth, $attr)= @_;
# Needs support at dbh level to clear cache before complaining about # active children. The XS template code does this. Drivers not using # the template must handle clearing the cache themselves. my $cache = $drh->FETCH('CachedKids'); $drh->STORE('CachedKids', $cache = {}) unless $cache;
my @attr_keys = $attr ? sort keys %$attr : (); my $key = join "~~", $dsn, $user||'', $auth||'', $attr ? (@attr_keys,@[EMAIL PROTECTED]) : (); my $dbh = $cache->{$key}; if ($dbh && $dbh->FETCH('Active') && eval { $dbh->ping }) { # XXX warn if BegunWork? # XXX warn if $dbh->FETCH('AutoCommit') != $attr->{AutoCommit} ? # but that's just one (bad) case of a more general issue. return $dbh; } $dbh = $drh->connect(@_); $cache->{$key} = $dbh; # replace prev entry, even if connect failed return $dbh; }
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()?
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.
Regards,
David
smime.p7s
Description: S/MIME cryptographic signature