On Sat, Jun 14, 2003 at 05:47:53PM +0200, Bjoern Kriews wrote:
> Hi !
>
> Following the DBI docs a coworker of mine tried to modify the return
> value
> ($_[2]) in a HandleError sub to implement automatic retry on error.
Oooh, scary.
> This works as documented with DBI::ExampleP and DBD::mysqlPP but not
> with DBD::mysql (and presumably all Driver.xst based DBDs).
>
> DBD::mysql uses DBIs Driver.xst Template to build itself and therefore
> uses (among others) XST_mUNDEF to return an error condition.
> This return value has the READONLY flag set and is aliased into the
> HandleError-subs @_ by XS_DBI_dispatch.
>
> Apparently the $_[2] access in the handler silently creates a local copy
> to work on, while I would expect something like
> "attempt to modify read-only value". (Why not ?)
>
> My XS-knowledge is _very_ limited but I tried the following patch
> (to the execute method only) and it fixes our testcase:
>
> --- DBI-1.37/Driver.xst Tue May 13 16:00:10 2003
> +++ DBI-1.37-test/Driver.xst Sat Jun 14 12:49:44 2003
> @@ -456,7 +456,7 @@
> if (retval == 0) /* ok with no rows affected */
> XST_mPV(0, "0E0"); /* (true but zero) */
> else if (retval < -1) /* -1 == unknown number of rows */
> - XST_mUNDEF(0); /* <= -2 means error */
> + ST(0) = sv_newmortal(); /* <= -2 means error */
> else
> XST_mIV(0, retval); /* typically 1, rowcount or -1 */
>
>
> IMHO this is a slight problem with the HandleError interface,
> I would suggest that XS_DBI_dispatch makes sure that READONLY SVs
> on the return stack are replaced by a writable copy
> before passing them to the handler, otherwise this will happen
> again.
>
> Can someone with more experience please comment on this ?
I agree entirely.
> Testcase follows:
Thanks. I've no time to test right now, can you try the attached
patch and let me know how it goes?
And I'd be interested in more details about the automatic-retry
mechanism you're using it for.
Tim.
p.s. The ig.co.uk address you used is a few years out of date.
--- DBI.xs 2003/06/16 08:59:22 11.30
+++ DBI.xs 2003/06/16 09:17:02
@@ -2632,34 +2632,43 @@
&& hook_svp && SvOK(*hook_svp)
) {
dSP;
- SV *result = *(sp-outitems+1);
PerlIO *logfp = DBILOGFP;
IV items;
SV *status;
+ SV *result; /* point to result SV that's pointed to by the stack */
+ if (outitems) {
+ result = *(sp-outitems+1);
+ if (SvREADONLY(result)) {
+ *(sp-outitems+1) = result = sv_2mortal(newSVsv(result));
+ }
+ }
+ else {
+ result = sv_newmortal();
+ }
if (debug)
PerlIO_printf(logfp," -> HandleError on %s via %s%s%s%s\n",
neatsvpv(h,0), neatsvpv(*hook_svp,0),
- (result ? " (" : ""),
- (result ? neatsvpv(result,0) : ""),
- (result ? ")" : "")
+ (!outitems ? "" : " ("),
+ (!outitems ? "" : neatsvpv(result ,0)),
+ (!outitems ? "" : ")")
);
PUSHMARK(SP);
XPUSHs(msg);
XPUSHs(sv_2mortal(newRV((SV*)DBIc_MY_H(imp_xxh))));
- XPUSHs( result ? result : sv_newmortal() );
+ XPUSHs( result );
PUTBACK;
items = perl_call_sv(*hook_svp, G_SCALAR);
SPAGAIN;
- status = POPs;
+ status = (items) ? POPs : &sv_undef;
PUTBACK;
if (!SvTRUE(status)) /* handler says it didn't handle it, so... */
hook_svp = 0; /* pretend we didn't have a handler... */
if (debug)
PerlIO_printf(logfp," <- HandleError= %s%s%s%s\n",
neatsvpv(status,0),
- (result ? " (" : ""),
- (result ? neatsvpv(result,0) : ""),
- (result ? ")" : "")
+ (!outitems ? "" : " ("),
+ (!outitems ? "" : neatsvpv(result,0)),
+ (!outitems ? "" : ")")
);
}
