Re: AutoCommit and connect_cached()

[David Wheeler]

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