On Wed, Mar 19, 2003 at 12:36:28AM -0500, Rudy Lippan wrote:
> On Tue, 18 Mar 2003, Tim Bunce wrote:
>
>
> > > 2. When you get a chance, could you send me your column info patch, so
> > > that I can apply it before mucking too with the perl code?
> >
> > Attached.
>
> Can you resend? I did not seem to get the attachment?
Oops, forgot. Attached. I've tweaked it slightly to use quote_identifier(),
which I'm presuming you're supporting (probably via adding get_info() data).
Some of the CHAR_SET_* and COLLATION_* fields could possibly be
added for mysql v4.1.
> > > Also looking at the auto reconenct, I think I can see why it defaults to
> > > on, but for the life of me, I don't understand why this 'fix' was put into
> > > the driver instead of telling people to fix their code. Grrr.
> >
> > I can. It's a very useful feature for some environments like mod_perl.
> > If the database server has been bounced you don't want the next few
> > requests (one for each child httpd) to fail.
>
> I try to make my code robust against such things; I ping the db and
> reconnect if needed.
Doing it in the driver for all server interactions is a more 'robust'
approach as it's impractical to do so it completely in the application.
> > > Anyway, I
> > > have a patch to add the mysql_auto_reconnect Attrubite. I left it
> > > defaulting to on, however, because of the existing code that depnds on the
> > > feature
> >
> > Fine. A way to get a count of how many auto reconnects have been
> > made would be handy, but not important for me as I'll probably just
> > disable it.
>
> I am doing the counts, but as for reporting the stats I was thinking more
> something like
>
> %stats = %{$dbh->{mysql_STATS}};
>
> $recn = $stats->{auto_reconnects};
>
> So we don't end up polluting the attributes with statistics if the number
> grows. Thoughts?
You're assuming the number will grow significantly :) I'd keep an IV in
the imp_dbh struct and intercept a FETCH of 'mysql_auto_reconnects'.
In future a FETCH of 'mysql_dbh_stats' could trigger construction of
a hash with a bunch of stats and return a ref to that.
Um, looks like you're doing roughly that in your patch.
> > It certainly needs to be made very clear that any use of LOCK TABLE
> > or GET_LOCK with DBD::mysql is currently unsafe as the locks can
> > be lost without notice.
>
> Well it looks like you will also get the magic reconnect with AutoCommit
> off too. mysql_real_connect() sets mysql.reconnect = 1, so the mysql
> libraries will automatically reconnect to the database on execute if
> needed?? Since mysql_real_execute() would always work, the DBD::mysql
> auto reconnect code was not being exercised (at least against
> 4.0.11-gamma.)
>
>
> Rudy
>
> ps. I have attached a first draft ofn the auto_reconnect patch (I want to
> do some more testing before I check it into CVS). The patch also fixes a
> problem where srings were not being quoted if they had been used in
> numeric context before passed to execute().
By forcing all to SQL_VARCHAR. That's worth noting in the docs
because (I think) some mysql SQL expressions/functions behave
differently if given a number or a string.
> @@ -1047,11 +1056,11 @@
> char *key = SvPV(keysv, kl);
> SV *cachesv = Nullsv;
> int cacheit = FALSE;
> + bool newval = SvTRUE(valuesv);
newval doesn't seem like a good name for this. bool_value perhaps.
> @@ -1091,6 +1100,12 @@
> + } else if (strlen("mysql_auto_reconnect")
> + == kl && strEQ(key,"mysql_auto_reconnect") )
Are you presuming the compiler will optimize strlen("mysql_auto_reconnect")
at compile time? I'd use kl==20 instead.
> + {
> + /*XXX: Does DBI handle the magic ? */
> + imp_dbh->auto_reconnect = newval;
What magic, specifically?
> @@ -1157,6 +1172,10 @@
> }
>
> switch(*key) {
> + case 'a':
> + if (kl == strlen("auto_reconnect") && strEQ(key, "auto_reconnect"))
> + result = sv_2mortal(newSViv(imp_dbh->auto_reconnect));
> + break;
Should have mysql_ prefix.
Tim.
