Firstly, apologies for starting a thread and then being too busy to follow it up - I really have been very busy but I would like to make some progress with this.

On 10/01/2011 22:35, Tim Bunce wrote:
On Mon, Jan 03, 2011 at 02:50:21PM +0000, Martin J. Evans wrote:
On 03/01/2011 13:08, Tim Bunce wrote:
Generally "method attributes" and "handle attributes" are quite
distinct. The one place they're not is the connect() methods that create
a statement handle.

It's an unfortunate historical accident that methods returning statement
handles can't also take handle attributes in the way you describe.
If you're interested in tackling this (for which I'd be delighted) then
we should start by defining the semantics and considering any backwards
compatibility issues.
Well, I seem to keep hitting this problem so I guess I am interested.
Great. Let's hope it all goes smoothly. We'll see...

my $sth = $dbh->prepare($sql, {ChopBlanks =>  1});
Yes, this is natural and would mirror the connect(..., { ... })
behaviour. In other words "create the handle then apply attributes".
Something like:

     sub prepare {
         my ($dbh, $sql, $attr) = @;

         my $sth = ...create new sth using $sql as now...

         # new logic:
         if ($attr) {
             $sth->{$_} = $attr->{$_} for keys %$attr;
         }
     }

Excellent, this would be perfect.

As for backward compatibility issues I'm not sure if there are any -
sounds like you might know something here I don't.
A tricky compatibility issue here is that some drivers already have "method
attributes" for prepare, DBD::Oracle for example has a long list
http://search.cpan.org/~pythian/DBD-Oracle/Oracle.pm#Prepare_Attributes
Such drivers would likely throw errors if the DBI started blindly
prepare method attributes like ora_parse_lang as handle attributes.
(Though some may already me statement attributes as well, I didn't check.)

Many of DBD::Oracle method attributes are really only that, method attributes (especially in prepare). In fact my branch of DBD::Oracle (not merged with trunk yet) removes many of the method attributes from private_attribute_info - a long standing bug which also causes me problems. Perhaps private_attribute_info gives us a way in here, in that if the driver specifies the attribute in private_attribute_info it is a driver-specific handle attribute and can be set anyway.

A simple but kludgy approach to that is to spec that only non-private
attributes (ie Capitalised) will be applied automatically. The driver
would have to look for and handle any private handle attributes itself.

ok, mixed with private_attribute_info that might not be so bad especially as I thought mixed case attributes are reserved for DBI.

A tricky implementation issue is how to get this logic into the drivers.
DBD::Oracle's prepare method, for example, looks like this:

     sub prepare {
        my($dbh, $statement, @attribs)= @_;

        # create a 'blank' sth

        my $sth = DBI::_new_sth($dbh, {
            'Statement' =>  $statement,
            });

        # Call Oracle OCI parse func in Oracle.xs file.
        # and populate internal handle data.

        DBD::Oracle::st::_prepare($sth, $statement, @attribs)
            or return undef;

        $sth;
     }

I imagine most other drivers are similar. It's tempting to pass @attribs
through to DBI::_new_sth() but I don't think that's a good idea.
While neat in theory, in practice the DBI::_new_sth() code would have to
call $sth->STORE(...) before the driver has fully initialized the handle.
I'm quite sure that would cause problems.

ah. I'm not sure why you are "quite sure" this would cause problems.

So I think what's needed is for the drivers to add some extra code
just before returning the new $sth. That way we're sure the $sth is ready.
Perhaps something like:

        DBD::Oracle::st::_prepare($sth, $statement, @attribs)
            or return undef;

         $sth->set_prepare_attributes($attribs[0]) if $attribs[0];

        $sth;
     }

(The use of @attribs instead of $attribs isn't significant. I think it
was a kind of cheap future proofing at the time.)

Sadly, drivers would need to add a call to set_prepare_attributes() to
get the new behaviour. So we'd loose code portability between drivers.
If a module like DBIx::Class assumed it could set $sth attributes this
way it would fail for any drivers that hadn't been updated yet.
I'm open minded about how big an issue this is in practice.

I cannot speak for DBIx::Class guys but I imagine they would not like it. However, past restrictions should not stop the advancement of DBI. This starts to enter another area that bothers me when using DBI with many different DBDs and that is there are no capabilities. The usual route is for DBI to offer up some method, like execute_array (which you know I'm also very interested in right now) and a DBD can override it with its own implementation without the application knowing anything about it or even controlling which implementation gets used. That is fine whilst DBI and the DBD do it exactly the same but in practise they don't. It would be nice to add a DBD capabilities method so that applications can find out what the driver provides and DBI could even use it to work out (like in this case) if a DBD was updated to support handle attributes on method calls. I've not thought this through properly, just really putting down ideas. DBi implemented the getinfo (from ODBC) but as far as I can see there is no equivalent for between DBI and a DBD.


An alternative approach would be to have the DBI method dispatcher
call set_prepare_attributes() itself when prepare() returns true
and the prepare() call had attributes.  I'm slightly uncomfortable with
that approach but it would avoid the portability issue.

uncomfortable?
So, next we need to agree on what, exactly, set_prepare_attributes()
should do. It would be written in perl so we (you) can spec it in code.

my $res = $dbh->selectall_arrayref($sql, {ChopBlanks =>  1});
That's a little harder to swallow from a DBI spec point of view.

The concept for connect() and now prepare() is "for methods that return
a new handle, any method attributes are set as handle attributes on the
new handle" [a]. The above doesn't fit that simple description.

because we don't see the new handle? Or because any method attributes are sticky to the method and disappear when the handle goes out of scope and yet in this case the handle goes out of scope immediately. Or is it simply that the selectall_arrayref does not return a handle? I suspect I'm making this too complicated and it is simply it does not return a handle and so does not fit the spec.
We'd need to extend it to something like "for methods that _create_
a new handle, any method attributes are set as handle attributes on the
new handle" [b].

Now I'm liking it more.
Okay, not a very big change. But then we find cases like this:

   $dbh->selectcol_arrayref($sql, { ChopBlanks =>  1, Columns =>  [2] });

Umm.

So now we need to extend it to something like "for methods that _create_
a new handle, any method attributes are set as handle attributes on the
new handle except for any documented attributes that really are method
attributes not handle attributes, which we'lll document and promise that
we'll never use the same name for both a handle attribute and a method
attribute" [c].

Umm.

But we know the supported handle attributes from private_attribute_info - don't we?
So now I'm wondering if something like this is a better idea:

   $dbh->selectcol_arrayref($sql, { Columns =>  [2], Prepare =>  { ChopBlanks 
=>  1 }})

In other words, have an explicit 'Prepare' attribute for passing
attributes to prepare(). That keeps the handle attributes separate from
the method attributes and means we can use definition [b] and avoid the
madness of [c].

Not quite what I was looking for but I guess I could live with it. Rather depends on whether you think we could use private_attribute_info.
That's neat but also has a portability issue in that the existing
select*_* methods all pass $attr direct to prepare().
Perhaps we can work around that by doing something like:

     $dbh->prepare($stmt, $attr->{Prepare} || $attr);

Umm. Ugly but seems workable.

Tim.
Martin

Reply via email to