Re: TYPE attribute in DBD::File related DBD's

On 30/07/13 21:30, H.Merijn Brand wrote:
> On Tue, 30 Jul 2013 20:23:50 +0100, "Martin J. Evans"
> <[email protected]> wrote:
>
>> On 30/07/2013 10:53, H.Merijn Brand wrote:
>>> The TYPE attribute is defined to be NUMERIC. Several databases do not
>>> follow that definition, like SQLite and CSV
>>>
>>> Luckily,  we control both DBD::CSV and the underlying DBD::File, se I
>>> can "fix" that for at least DBD::CSV and probably all DBD's using the
>>> DBD::File layer. I propose this change:
>>>
>>> --8<---
>>> diff --git a/lib/DBD/File.pm b/lib/DBD/File.pm
>>> index 444c4d4..ffc5e84 100644
>>> --- a/lib/DBD/File.pm
>>> +++ b/lib/DBD/File.pm
>>> @@ -280,6 +280,7 @@ my %supported_attrs = (
>>>        PRECISION => 1,
>>>        NULLABLE  => 1,
>>>        );
>>> +my $type_map;
>>>
>>>    sub FETCH
>>>    {
>>> @@ -306,8 +307,23 @@ sub FETCH
>>>
>>>               my @colnames = $sth->sql_get_colnames ();
>>>
>>> +           unless ($type_map) {
>>> +               $type_map = {   # Minimal type set (like CSV)
>>> +                   BLOB    => -4,
>>> +                   TEXT    => -1,
>>> +                   CHAR    =>  1,
>>> +                   INTEGER =>  4,
>>> +                   REAL    =>  7,
>>> +                   VARCHAR => 12,
>>> +                   };
>>> +               my $tia = $sth->{Database}->type_info_all ();
>>> +               # TYPE_NAME => DATA_TYPE
>>> +               $type_map->{$_->[0]} = $_->[1] for grep { ref $_ eq "ARRAY" } @$tia;
>>
>> I have not looked at this in context but it seems to me this code is
>> assuming element 0 and element 1 are TYPE_NAME and DATA_TYPE whereas
>> type_info_all starts with a map which describes the columns in the
>> result.
>
> Yes, «grep { ref $_ eq "ARRAY" }» filters that out
> It is a pseudo-hash, which I don't like, but it is unlikely to ever
> change. Would you be more comfortable with this?
>
> 	    unless ($type_map) {
> 	        # TYPE_NAME => DATA_TYPE
> 		$type_map = {	# Minimal type set (like CSV)
> 		    BLOB    => -4,
> 		    TEXT    => -1,
> 		    CHAR    =>  1,
> 		    INTEGER =>  4,
> 		    REAL    =>  7,
> 		    VARCHAR => 12,
> 		    };
> 		if (my $tia = $sth->{Database}->type_info_all ()) {
> 		    my $tiah = shift @$tia;
> 		    my %tiah = map { uc ($_) => $tiah->{$_} } keys %$tiah;
> 		    my ($tni, $dti) = map {$tiah->{$_}} "TYPE_NAME", "DATA_TYPE";
> 		    $type_map->{$_->[$tni]} = $_->[$dti] for @$tia;
> 		    }
> 		}
>
> works just as well for me

Yes.

>>> +               }
>>> +
>>>               $attr eq "TYPE"      and
>>> -               return [ map { $sth->{f_overall_defs}{$_}{data_type}   || "CHAR" }
>>> +               return [ map { $type_map->{$_} || $_ }
>>> +                        map { $sth->{f_overall_defs}{$_}{data_type}   || "VARCHAR" }
>>>                               @colnames ];
>>>
>>>               $attr eq "PRECISION" and
>>> -->8---
>>>
>>> All DBI tests still pass, and several TODO tests in DBD::CSV now pass
>>>
>>> If DBD::xxx provides GetInfo.pm and TypeInfo.pm, the conversions will
>>> automatically be updated (once).
>>
>> It will work as it stands so long as type_info_all maps type_name and
>> data_type as indexes 0 and 1. If that changes it looks like it will
>> break.
>
> The chances that will ever change are ZERO

True but how many times have I fixed a bug because some coder made the same assumption.

>> However, I might have misread this as I did not follow the context.
>>
>> Martin
>

Martin
-- 
Martin J. Evans
Easysoft Limited
http://www.easysoft.com