subject:"Re\: TYPE attribute in DBD\:\:File related DBD's"

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

2013-07-31 Thread Martin J. Evans


On 30/07/13 21:30, H.Merijn Brand wrote:

On Tue, 30 Jul 2013 20:23:50 +0100, Martin J. Evans
martin.ev...@easysoft.com 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

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

2013-07-31 Thread H.Merijn Brand

On Wed, 31 Jul 2013 08:51:40 +0100, Martin J. Evans
martin.ev...@easysoft.com wrote:

 On 30/07/13 21:30, H.Merijn Brand wrote:
  On Tue, 30 Jul 2013 20:23:50 +0100, Martin J. Evans
  martin.ev...@easysoft.com wrote:
  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.

Change pushed to dir_search_hashes branch in DBI
This is the branch where I am currently working in to support the
missing features for DBD::File related DBD's like DBD::CSV

I wanted a clean test case before serious DBI::Test play started

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

Assumption may well be the mother of all fuckups.
Knowing the underlying code however did make me know rather than assume
what I was doing, but as this is not *hot* code (it is only done once)
I agree with the portability change.

-- 
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

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
martin.ev...@easysoft.com 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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information