Re: confused about DBD implementation of ChopBlanks

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;
>          }
>      }
>
>> 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.)
>
> 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.
>
> 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.
>
> 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.
>
> 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.
>
> 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.
>
> 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].
>
> 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.
>
> 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].
>
> 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.
Tim,

Thanks for the time you've obviously spent on this. I am not ignoring 
you but I'm snowed under with $work and a rush of other OS stuff at the 
moment and I'll come back to it, hopefully within a week.

Martin