[perl5-dbi/dbi] 33b922: The TYPE attribute should be NUMERIC, even in DBD:...

2013-07-30 Thread H.Merijn Brand - Tux
  Branch: refs/heads/dir_search_hashes
  Home:   https://github.com/perl5-dbi/dbi
  Commit: 33b92211c016edaf2c826b464018b9cdea3e3b46
  
https://github.com/perl5-dbi/dbi/commit/33b92211c016edaf2c826b464018b9cdea3e3b46
  Author: H.Merijn Brand - Tux 
  Date:   2013-07-30 (Tue, 30 Jul 2013)

  Changed paths:
M Changes
M lib/DBD/File.pm

  Log Message:
  ---
  The TYPE attribute should be NUMERIC, even in DBD::File





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

2013-07-30 Thread H.Merijn Brand
On Tue, 30 Jul 2013 20:23:50 +0100, "Martin J. Evans"
 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

> > +   }
> > +
> >  $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

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

-- 
H.Merijn Brand  http://tux.nl   Perl Monger  http://amsterdam.pm.org/
using perl5.00307 .. 5.19   porting perl5 on HP-UX, AIX, and openSUSE
http://mirrors.develooper.com/hpux/http://www.test-smoke.org/
http://qa.perl.org   http://www.goldmark.org/jeff/stupid-disclaimers/


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

2013-07-30 Thread Martin J. Evans

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.



+   }
+
 $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.

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

Martin



TYPE attribute in DBD::File related DBD's

2013-07-30 Thread H.Merijn Brand
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;
+   }
+
$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).

-- 
H.Merijn Brand  http://tux.nl   Perl Monger  http://amsterdam.pm.org/
using perl5.00307 .. 5.19   porting perl5 on HP-UX, AIX, and openSUSE
http://mirrors.develooper.com/hpux/http://www.test-smoke.org/
http://qa.perl.org   http://www.goldmark.org/jeff/stupid-disclaimers/