On 06/02/2011 11:16, H.Merijn Brand wrote:
On Sat, 5 Feb 2011 12:29:09 +0000, Tim Bunce<tim.bu...@pobox.com>
wrote:

On Fri, Feb 04, 2011 at 08:00:13PM +0000, Martin J. Evans wrote:
Tim,

At the LPW I think I promised to add connection and encoding trace
flags to DBI which add to the already existing SQL flag. I never got
around to it and Merijn reminded me today on #dbi. The change is
trivial and once implemented I will replace DBD::ODBC trace flags
for connection and encoding with the standard DBI ones. There only
remains 2 small issues wrt to this which need confirming:

1. DBI currently says
     return 0x00000100 if $name eq 'SQL';

    and as there is no macro for 0x00000100 I have the following in
DBD::ODBC:

/* Can't find a constant in DBI for SQL tracing but it is 256 */
#define SQL_TRACE_FLAG 0x100

and then code like this:

    if (DBIc_TRACE(imp_dbh, SQL_TRACE_FLAG, 0, 3)) {
        TRACE1(imp_dbh, "    SQLExecDirect %s\n", SvPV_nolen(statement));
    }

Should there be macros for DBI's trace flags
Yes. I'd suggest

     #define DBIf_TRACE_SQL   0x00000100

and if so where do they go (in DBIXS.h?)
Just above DBIc_TRACE_MATCHES.

2. I propose adding connection and encoding flags to DBI - so far
they've kept pretty short:

   ALL
   SQL

so I've replicated this with:

     sub parse_trace_flag {
     my ($h, $name) = @_;
     #      0xddDDDDrL (driver, DBI, reserved, Level)
     return 0x00000100 if $name eq 'SQL';
     return 0x00000200 if $name eq 'CON';
     return 0x00000400 if $name eq 'ENC';
     return;
     }

and:

Currently the DBI only defines four trace flags:

   ALL - turn on all DBI and driver flags (not recommended)
   SQL - trace SQL statements executed
         (not yet implemented in DBI but implemented in some DBDs)
   CON - trace connection process
         (not yet implemented in DBI but implemented in some DBDs)
   ENC - trace encoding (unicode translations etc)
         (not yet implemented in DBI but implemented in some DBDs)

is this ok?
I'm not a huge fan of short names in general, but in this case I think
it's reasonable.

and 2 supplementals:

3. I could look into implementing some/all those flags in DBI but it
seems these areas are pretty much covered with trace levels right
now
I think it's worth adding CON. It would enable connect and disconnect to
be traced *with no other output*.

and in any case, I cannot find prepare in DBI (for SQL tracing)
anyway.
The trace flag checking would need to be applied in XS_DBI_dispatch
and controlled by the 'Internal Method Attributes' (dbi_ima_st).

I'm thinking it would work along these lines:

     When a method with the CON trace flag is called and the current
     trace setting on the handle has the CON trace flag set, then the trace
     level will be set to min(trace_level,2) for the duration of the call.

Sound ok?

(I've partly implemented that but not tested it yet.)

4. Lastly (and not wanting to get in Merijn's way who agreed to look
at it) but at the same workshop you discussed dbd_verbose (or
whatever some DBDs have added support for) with Merijn and we think
you said there is space in the trace settings for DBD trace
settings. Apparently some DBDs only want DBD tracing and not DBI
tracing so trace level is no good as it always includes DBI tracing.
I think you were talking about reserving some space for DBDs only -
perhaps you could remind us - the idea being settings (or a setting)
which outputs DBD tracing but not DBI tracing.
As you note above, the current trace settings are encoded into an int:

       #      0xddDDDDrL (driver, DBI, reserved, Level)

Where L is the DBI trace level (0-16) and the rest are bit flags.
The 4 reserved bits could be used as a driver trace level (0-16).
#   0xddlDDDrL (driver, driver-level, DBI, reserved, Level)

would be perfect in my world.

Why I started the whole debug discussion so long ago, was that I by now
trust the way DBI does handle what it has to handle, and 95% of the
time I run into trouble, I just want to see what the driver (DBD) is
doing in the back.

So far I didn't get all the authors into a straight line. We all had
our own vision in what was most useful to debug our own DBD without
looking at how other DBD's did their debugging, and I must say I am
just as guilty.

Now back to that "need", I invented $DBD_VERBOSE =>  $dbh->{dbd_verbose}
to skip all the DBI messages (as said, they only caused noise to what I
wanted to see).

back to 0xddlDDDrL

l than is "level" to DBD, what "L" is to DBI

How many bits in DDDD do you think will ever be used? If we now use 2
or 3, wouldn't it be better to change to

#  0xddlDDDrL ?

Which would mean that we stay 100% backward compatible (as the first D
of DDDD was only reserved but never used.

With this scheme, it is extremely easy for all drivers to update their
docs to match DBI. Those drivers that used a level will just have to
use the 'l', those that used flags will (still) have to use the dd.

We could then document the use and support of DBD_VERBOSE to
automatically translate to the ddl part. If a DBD already supported
$DBD_VERBOSE instead of "ddl", it will work just as it did, if the DBD
updates to the new scheme (requiring DBI-1.xxx that supports it), then
the "upgrade" is automatic and doesn't change a thing from the user
point of view.

Am I being messy? Or does this all make sense?

That's fine by me Merijn. My objection was adding dbd_verbose in addition to the existing flags meant another test every time I wanted to trace and I wanted to avoid that - which the 'DBD' flag does for me. I have no problem with what other DBDs do but I think it would be nice to try to get them all inline at some stage. Your suggestion does not seem to harm anything and allows dbd_verbose implemented in some drivers to map to something inside DBI. You could even set all of 'l' bits if the 'DBD' flag was set so drivers which work on 'l' but not the flag would behave the same.

So your only remaining problem is how to set 'l' on the command line or through the various tracelevel calls or were you thinking dbd_verbose = n would just set 'l' = n?

As far as I can see few DBD's even use DBIc_TRACE, most seem to just test the trace level - would be nice to change that.

Martin

Reply via email to