Re: [rt.cpan.org #61168] Specifying seperation character per table does not work

Jens Rehsack <[email protected]>
Newsgroups gmane.comp.lang.perl.modules.dbi.sybase.devel
Message-ID <[email protected]>
2010/9/14 Jens Rehsack <[email protected]>:
> 2010/9/14 H.Merijn Brand via RT <[email protected]>:
>> <URL: https://rt.cpan.org/Ticket/Display.html?id=61168 >
>>
>> The "problem" is two-fold. Using the most recent versions of DBI,
>> SQL::Statement, Text::CSV_XS and DBD::CSV, the problem was that the
>> csv_in and csv_out objects for Text::CSV_XS were created before they
>> saw the sep_char attribute change, so it never got to the parser.
>
> That's what I tried to explain at the begin of July - but I know, Belgium Perl
> Workshop and holiday preparation...
>
>> I've patched the most recent version (get it from git to test).
>
> The last paragraph of
> http://search.cpan.org/~timb/DBI-1.613_94/lib/DBD/File/HowTo.pod#User_comfort
> should you guide how it's intended.

Ok, I understood your desire - but your patch doesn't catch all the backward
compatibility you typically want - see (and apply)
0001-add-test-case-for-rt61168.patch
first.

As we have no reason this time to run DBD::CSV-0.31 with DBI<1.614, I
didn't patch
the set_table_meta_attr method but added the real fix using newly introduced
table_meta_attr_changed (0002-fix-rt61168.patch - requires patch-DBI-rt61168).

If any of the DBI developers wants to review patch-DBI-rt61168 before I commit,
please do.

>> Note that you cannot pass it as "sep_char", but have to add the DBD
>> prefix and use "csv_sep_char".
>
> The documentation should be updated in this case.
>
>> $ cat sandbox/rt61168.pl
> [...]
>> Jens, note the wrong setting for f_fqbn
>
> I'd say this is intended by the file2table implementation, but I
> can't figure out why. We should probably take a close look
> to this function together :(
>
> Stop timbo releasing 1.614 before we figured out this. Probably we
> should add a nice test for it to t/51dbm_file.t ...

This should be the next task and all apologies to David "theory" Wheeler
for delaying the release.

/Jens
0001-add-test-case-for-rt61168.patch (application/octet-stream, 1.7 KB)
From 77497101cc470201c66c094d5192dbab193f3577 Mon Sep 17 00:00:00 2001
From: Jens Rehsack <[email protected]>
Date: Wed, 15 Sep 2010 23:38:04 +0200
Subject: [PATCH 1/2] add test case for rt61168

---
 t/80_rt.t |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/t/80_rt.t b/t/80_rt.t
index 66da2d7..c8b6a51 100644
--- a/t/80_rt.t
+++ b/t/80_rt.t
@@ -232,6 +232,37 @@ while (<DATA>) {
     ok ($dbh->disconnect,			"disconnect");
     }
 
+{   $rt = 61168;
+    ok ($rt, "RT-$rt - $desc{$rt}");
+    my @lines = @{$input{$rt}};
+
+    open  FILE, ">output/rt$rt";
+    print FILE @lines;
+    close FILE;
+
+    ok (my $dbh = Connect ({ f_lock => 0 }),				"connect");
+    $dbh->{csv_tables}{rt61168}{sep_char} = ';';
+    cmp_ok ($dbh->{csv_tables}{rt61168}{csv_in}{sep_char}, "eq", ";",	"cvs_in adjusted");
+    cmp_ok ($dbh->{csv_tables}{rt61168}{csv_out}{sep_char}, "eq", ";",	"cvs_out adjusted");
+    ok (my $sth = $dbh->prepare ("select * from rt$rt"),		"prepare");
+
+    ok ($sth->execute (),						"execute");
+    ok (my $all_rows = $sth->fetchall_arrayref({}),			"fetch");
+    my $wanted_rows = [
+	{
+	    header1 => "Volki",
+	    header2 => "Bolki",
+	    },
+	{
+	    header1 => "Zolki",
+	    header2 => "Solki",
+	    },
+	];
+    is_deeply ($all_rows, $wanted_rows,		"records");
+
+    ok ($dbh->do ("drop table RT$rt"),		"drop");
+    ok ($dbh->disconnect,			"disconnect");
+    }
 done_testing ();
 
 __END__
@@ -266,3 +297,7 @@ c.tab,"s,tab"
 «46627» - DBD::File is damaged now
 «51090» - Report a bug in DBD-CSV
 integer,longvarchar,numeric
+«61168» - Specifying seperation character per table does not work
+"HEADER1";"HEADER2"
+Volki;Bolki
+Zolki;Solki
-- 
1.7.0.5
patch-DBI-rt61168 (application/octet-stream, 3.4 KB) - not displayed
0002-fix-rt61168.patch (application/octet-stream, 1.6 KB)
From a026eac1dd98624da3434d04e1aaae40d9d549ea Mon Sep 17 00:00:00 2001
From: Jens Rehsack <[email protected]>
Date: Wed, 15 Sep 2010 23:38:49 +0200
Subject: [PATCH 2/2] fix rt61168

---
 lib/DBD/CSV.pm |   20 ++++----------------
 1 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/lib/DBD/CSV.pm b/lib/DBD/CSV.pm
index 7e5c6fd..c067a93 100755
--- a/lib/DBD/CSV.pm
+++ b/lib/DBD/CSV.pm
@@ -375,24 +375,13 @@ sub init_table_meta
     } # init_table_meta
 
 my %compat_map = (
-    file  => "f_file",
     map { $_ => "csv_$_" } qw( class eof  eol quote_char sep_char escape_char )
     );
+__PACKAGE__->register_compat_map (\%compat_map);
 
-sub get_table_meta_attr
-{
-    my ($class, $meta, $attr) = @_;
-    defined $compat_map{$attr} and
-        return $class->SUPER::get_table_meta_attr ($meta, $compat_map{$attr});
-    return $class->SUPER::get_table_meta_attr ($meta, $attr);
-    } # get_table_meta_attr
-
-sub set_table_meta_attr
+sub table_meta_attr_changed
 {
     my ($class, $meta, $attr, $value) = @_;
-    defined $compat_map{$attr} and
-        return $class->SUPER::set_table_meta_attr ($meta, $compat_map{$attr}, $value);
-
     (my $csv_attr = $attr) =~ s/^csv_//;
     if (exists $csv_xs_attr{$csv_attr}) {
 	for ("csv_in", "csv_out") {
@@ -400,9 +389,8 @@ sub set_table_meta_attr
 		$meta->{$_}{$csv_attr} = $value;
 	    }
 	}
-
-    return $class->SUPER::set_table_meta_attr ($meta, $attr, $value);
-    } # set_table_meta_attr
+    $class->SUPER::table_meta_attr_changed ($meta, $attr, $value);
+    } # table_meta_attr_changed
 
 $DBD::File::VERSION > 0.38 and *open_file = sub {
     my ($self, $meta, $attrs, $flags) = @_;
-- 
1.7.0.5
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.