Re: promised trace flags change - a bit late

On 05/02/2011 12:54, Martin J. Evans wrote:
> On 05/02/2011 12:34, Martin J. Evans wrote:
>> On 05/02/2011 12:29, Tim Bunce 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
>>>
>> ok
>>>> and if so where do they go (in DBIXS.h?)
>>> Just above DBIc_TRACE_MATCHES.
>>>
>> ok
>>>> 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.
>>>
>> neither am I but on the DBI_TRACE=x|y|z line it is nicer for them to 
>> be short.
>>>> 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*.
>>>
>> ok
>>>> 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.)
>>>
>> this is an area I am not that familiar with - bit out of my depth 
>> here at the moment. Similar issue with handle attributes and method 
>> attributes discussion - I understood what you said but am struggling 
>> to implement it.
>>>> 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).
>>>
>>> Tim.
>>>
>> I'll leave that bit for Merijn as this seemed to be something he 
>> wanted/needed.
>>
>> I'll check in the changes I put forward in the next hour.
>>
>> Martin
>
> ok, the 3 new trace flags are in and I added macros (slight shame you 
> cannot use them in DBI.pm too but not a great deal). I've also 
> implemented the new DBI trace flags in DBD::ODBC and am testing now. 
> Because DBD::ODBC already had ENC and CON trace flags I've simply ored 
> then in DBD::ODBC with DBI's new ones so you can use either for now 
> then some time after DBI is released I will remove the DBD::ODBC ones.
>
> I've also changed all DBD::ODBC tracing to look at DBIf_TRACE_DBD so 
> you should be able to get only DBD tracing - still to test.
>
> Martin

All done in DBD::ODBC, tested and committed to trunk for next release.

For other DBD authors following this (and assuming it makes the next DBI 
release without modification) you can now replace your dbd_verbose 
attribute with:

    if (DBIc_TRACE(imp_xxh, DBIf_TRACE_DBD, 0, 3)) {

which will be true when trace level is > 3 or when someone set the DBD 
trace flag. You can see DBD::ODBC's dbdimp.c for how I did it as until 
1.617 of DBI is released the DBIfxxx flags will not exist so I worked 
around that for the situation when pre DBI 1.617 is used.

The only small issue I have is a small amount of DBI tracing is output 
right at the start even when DBD tracing only is set:

0x2a00 DBI::db::get_info
0x10000 DBI::db::take_imp_data
0x10c00 DBI::db::disconnect
0x2000 DBI::db::selectrow_array
0x2200 DBI::db::tables
0x0430 DBI::db::quote_identifier
0x0000 DBI::db::clone
0x0430 DBI::db::quote
0x2200 DBI::db::type_info
.
.

Martin