On Wed, 25 Apr 2007 09:54:34 +0100, Tim Bunce <[EMAIL PROTECTED]> wrote:
> On Mon, Apr 23, 2007 at 08:56:45PM -0400, Patrick Galbraith wrote:
> > Hi all,
> >
> > Soon, I'll be releasing DBD::mysql 4.005 and was wondering if there any
> > last minute thoughts, requests?
>
> A few general 'DBI best practice' items (that other drivers authors
> should check their code for)...
Good for immediate checking :)
Some short questions to check if I understood that all right ...
> ---
> Don't use DBIS or dbis when you can use DBIc_STATE (imp_xxh).
> DBIS is much slower, especially on perls built with threads enabled.
> It's a macro that expands to a function call, whereas
> DBIc_STATE (imp_xxh) is just accessing a field in the handle structure.
This does not apply to?:
av = DBIS->get_fbav (imp_sth);
num_fields = AvFILL (av) + 1;
> ---
>
> In the specific case of dbis->debug or DBIS->debug, use
> DBIc_TRACE_LEVEL (imp_xxh) instead.
What if the debug function that uses dbis does not have a handle
in it's visible arguments?
> That correctly masks out the higher bits that DBI now reserves
> for trace flags. See TRACING in the DBI docs.
So, this:
for (i = 0; i < n; i++) {
imp_sth_t *imp_sth = children[i];
if (2 > dbis->debug && 2 > opt_v) {
would change to this?:
for (i = 0; i < n; i++) {
imp_sth_t *imp_sth = children[i];
if (2 > DBIC_TRACE_LEVEL (imp_sth) && 2 > opt_v) {
> ---
> DBIc_ERR and DBIc_ERRSTR, and DBIc_STATE should not be set directly.
> So change do_error () from the existing 28 lines to something like this:
>
> void do_error (SV *h, int rc, const char *what, const char *sqlstate)
> {
> D_imp_xxh (h);
> DBIh_SET_ERR_CHAR (h, imp_xxh, Nullch, rc, what, sqlstate, Nullch);
> }
>
> That will call set_err () to correctly merge info, warning and error states
> and trigger the $h->{HandleSetErr} hook. See set_err () in the DBI docs.
So, this:
static void error (SV *h, int error_num, char *text, char *state)
{
D_imp_xxh (h);
sv_setiv (DBIc_ERR (imp_xxh), (IV)error_num);
sv_setpv (DBIc_ERRSTR (imp_xxh), text);
unless (state)
sv_setpv (DBIc_STATE (imp_xxh), state);
} /* error */
would change to this?:
static void error (SV *h, int error_num, char *text, char *state)
{
D_imp_xxh (h);
DBIh_SET_ERR_CHAR (h, imp_xxh, Nullch, error_num, text, state, Nullch);
} /* error */
and still do the sam
> ---
> The do_warn code is confused. It sets DBIc_ERR to a true value which
> means it's really acting like do_error (it'll trigger RaiseError etc).
> It should probably look like this:
>
> void do_warn (SV *h, int rc, char *what)
> {
> D_imp_xxh (h);
> DBIh_SET_ERR_CHAR (h, imp_xxh, "0", 0, what); /* "0" = warning */
> }
I have no do_warn () code in my DBD. Should I have?
> ---
> doquietwarn doesn't seem to be used, but if it was then
> DBIc_WARN (imp_xxh) could be used as the control. That defaults to true
> but can be turned off via $h->{Warn}=0. It's inherited by child handles.
--
H.Merijn Brand Amsterdam Perl Mongers (http://amsterdam.pm.org/)
using & porting perl 5.6.2, 5.8.x, 5.9.x on HP-UX 10.20, 11.00, 11.11,
& 11.23, SuSE 10.0 & 10.2, AIX 4.3 & 5.2, and Cygwin. http://qa.perl.org
http://mirrors.develooper.com/hpux/ http://www.test-smoke.org
http://www.goldmark.org/jeff/stupid-disclaimers/
