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