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" 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" wrote: > On 30/07/13 21:30, H.Merijn Brand wrote: > > On Tue, 30 Jul 2013 20:23:50 +0100, "Martin J. Evans" > > 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/
[perl5-dbi/DBI-Test] a2d1b2: create separate tests for DBI and DBI::Mock ...
Branch: refs/heads/master Home: https://github.com/perl5-dbi/DBI-Test Commit: a2d1b22d134be0ca353e0f0b312fb14da24c798e https://github.com/perl5-dbi/DBI-Test/commit/a2d1b22d134be0ca353e0f0b312fb14da24c798e Author: Jens Rehsack Date: 2013-07-31 (Wed, 31 Jul 2013) Changed paths: M lib/DBI/Test/Conf.pm Log Message: --- create separate tests for DBI and DBI::Mock ... create separate tests for DBI and DBI::Mock only when DBI is found
[perl5-dbi/DBI-Test] 1cbb32: mock installed_/available_drivers, too
Branch: refs/heads/master Home: https://github.com/perl5-dbi/DBI-Test Commit: 1cbb32379a6ac05d6a16a6e58d66e2be1c26a6fb https://github.com/perl5-dbi/DBI-Test/commit/1cbb32379a6ac05d6a16a6e58d66e2be1c26a6fb Author: Jens Rehsack Date: 2013-07-31 (Wed, 31 Jul 2013) Changed paths: M lib/DBI/Mock.pm Log Message: --- mock installed_/available_drivers, too but only have DBD::NullP - nothing else
[perl5-dbi/DBI-Test] e8af40: extend prefix generation by driver based dsn
Branch: refs/heads/master Home: https://github.com/perl5-dbi/DBI-Test Commit: e8af4011e2b734b96d3c1c13c73b970385e8de44 https://github.com/perl5-dbi/DBI-Test/commit/e8af4011e2b734b96d3c1c13c73b970385e8de44 Author: Jens Rehsack Date: 2013-07-31 (Wed, 31 Jul 2013) Changed paths: M lib/DBI/Test/Conf.pm Log Message: --- extend prefix generation by driver based dsn Since it makes sense to decouple 1600 tests in SQL-Statement/t/06virtual.t by separating it into one file per used DBD, or clean up DBI/t/52dbm_complex.t, we now have the opportunity to do that. Pitfall - it defaultly generates all variants even when they are not necessary (eg. for connection test, it doesn't connect better with MLDBM or BerkeleyDB).
[perl5-dbi/DBI-Test] db6be1: fix minor nits
Branch: refs/heads/master Home: https://github.com/perl5-dbi/DBI-Test Commit: db6be10418f3b1051be6e7baba5aee17c4a6593d https://github.com/perl5-dbi/DBI-Test/commit/db6be10418f3b1051be6e7baba5aee17c4a6593d Author: Jens Rehsack Date: 2013-07-31 (Wed, 31 Jul 2013) Changed paths: M MANIFEST.SKIP Log Message: --- fix minor nits Commit: 0d7644cabf300c340641ba83d24dc0ed8aac0954 https://github.com/perl5-dbi/DBI-Test/commit/0d7644cabf300c340641ba83d24dc0ed8aac0954 Author: Jens Rehsack Date: 2013-07-31 (Wed, 31 Jul 2013) Changed paths: M MANIFEST Log Message: --- regenerate MANIFEST Commit: 16c0e14bf819209c3d1fafbceafa1497f5d5cb01 https://github.com/perl5-dbi/DBI-Test/commit/16c0e14bf819209c3d1fafbceafa1497f5d5cb01 Author: Jens Rehsack Date: 2013-07-31 (Wed, 31 Jul 2013) Changed paths: M Makefile.PL M lib/DBI/Test/Conf.pm Log Message: --- respect with of Tim Bunce that AUTHOR_TESTS will ... Respect with of Tim Bunce that AUTHOR_TESTS will be generated into xt/ but to t/ It's currently on MANIFEST.SKIP to care them not in distribution Compare: https://github.com/perl5-dbi/DBI-Test/compare/4ac7e155fcbf...16c0e14bf819