Skip Menu |
 

This queue is for tickets about the Class-DBI CPAN distribution.

Report information
The Basics
Id: 7715
Status: open
Priority: 0/
Queue: Class-DBI

People
Owner: Nobody in particular
Requestors: apeiron [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: (no value)
Fixed in: (no value)



Subject: Class::DBI does not correctly quote column names (Pg, maybe others)
Download (untitled) / with headers
text/plain 1.6k
When attempting to use a table which contains a reserved name as a column name, Class::DBI does not properly quote the column name when issuing SQL requests. The attached script provides this output when run against such a table: Can't insert new main: DBD::Pg::st execute failed: ERROR: syntax error at or near "user" at character 168 [for Statement "INSERT INTO users (ulbandwidth, password, dlbandwidth, uid, user, gid, dir) VALUES (?, ?, ?, ?, ?, ?, ?) "] at /usr/home/apeiron/software/perl/lib/site_perl/5.9.1/DBIx/ContextualFetch.pm line 51. Class::DBI::_croak('main=HASH(0x83f3e7c)', 'Can\'t insert new main: DBD::Pg::st execute failed: ERROR: s...', 'err', 'DBD::Pg::st execute failed: ERROR: syntax error at or near "...', 'method', 'create') called at /usr/home/apeiron/software/perl/lib/site_perl/5.9.1/Class/DBI.pm line 644 Class::DBI::_insert_row('main=HASH(0x83f3e7c)', 'HASH(0x83f3f78)') called at /usr/home/apeiron/software/perl/lib/site_perl/5.9.1/Class/DBI.pm line 590 Class::DBI::_create('main', 'HASH(0x83e8658)') called at /usr/home/apeiron/software/perl/lib/site_perl/5.9.1/Class/DBI.pm line 470 Class::DBI::create('main', 'HASH(0x83e8484)') called at class1 line 13 This works if I use straight DBI, Alzabo, or the psql command-line utility. It also works for my FTP server which uses this database to authenticate users. So, obviously, the problem lies somewhere in between Class::DBI and DBI. I've tried every single quote permutation of which I can think to try to solve this issue; cf. http://www.perlmonks.org/index.pl?node_id=392477 for further discussion.
Download class1
application/x-perl 803b

Message body not shown because it is not plain text.

Date: Tue, 21 Sep 2004 08:54:51 +0100
From: Tony Bowden <tony [...] kasei.com>
To: Christopher via RT <bug-Class-DBI [...] rt.cpan.org>
Subject: Re: [cpan #7715] Class::DBI does not correctly quote column names (Pg, maybe others)
RT-Send-Cc:
Download (untitled) / with headers
text/plain 308b
On Mon, Sep 20, 2004 at 05:56:26PM -0400, Christopher via RT wrote: Show quoted text
> This works if I use straight DBI
This makes me think it's not a CDBI problem, as CDBI doesn't do anything more or less than DBI here. Can you reduce this down to a small test script which passes on DBI and fails on CDBI? Thanks, Tony
Download (untitled) / with headers
text/plain 1.3k
[APEIRON - Tue Sep 21 10:00:31 2004]: Show quoted text
>> This makes me think it's not a CDBI problem, as CDBI doesn't do >> anything more or less than DBI here.
> Okay, but I can quote things with DBI. I've tried every quoting method > possible in Perl with CDBI, but none of them actually succeed with the > insert.
Ah, if you're explicitly quoting it with DBI that's a different matter. You're not going to be able to do this with CDBI. It assumes that all your column names are going to be sensible. To get this working isn't trivial, as it will require factoring out all the places that construct SQL from column names and calling a method on them to quote them ... which I assume is a database specific thing, so by default CDBI won't actually be able to do anything, and have to leave it to database specific subclasses to handle. It's should be too difficult to get most of them working fairly quickly just for yourself if you want to play around with it. If you're waiting for me it'll probably be about 6 months. You could also try the mailing list and see if anyone there has any time to help... The main places for the simple case (i.e. if you're not handling SQL expansions, remapped column names etc) are probably just _insert_row and _do_search, where you can replace the column variables with a $self->_quoted_column_name($col) which DTRT for you. Tony Tony
From: qef [...] ungwe.org
Download (untitled) / with headers
text/plain 1.1k
I had a go at fixing this, and ended up with the patch below. But I don't know how much of the problem this solves. It seems to work for the few things I've tried it with, but it would need some proper testing. Right now though I'll probably just rearrange my table and column names. qef --- lib/Class/DBI.pm.orig 2004-04-30 08:22:12.000000000 +0100 +++ lib/Class/DBI.pm 2005-05-04 08:28:20.734250373 +0100 @@ -169,10 +169,14 @@ my ($self, $sql, @args) = @_; my %cmap; + my $dbh = $self->db_Main; my $expand_table = sub { my ($class, $alias) = split /=/, shift, 2; my $table = $class ? $class->table : $self->table; $cmap{ $alias || $table } = $class || ref $self || $self; + for ($table, $alias) { + $_ = $dbh->quote_identifier($_) if defined; + } ($alias ||= "") &&= " AS $alias"; return $table . $alias; }; @@ -200,8 +204,9 @@ }; $self->_croak("Don't know how to join $c1 to $c2") unless $col; - push @sql, sprintf " %s.%s = %s.%s ", $t1, $col, $t2, - $c2->primary_column; + push @sql, sprintf " %s.%s = %s.%s ", + map { $dbh->quote_identifier } + $t1, $col, $t2, $c2->primary_column; } return join " AND ", @sql; };
Download (untitled) / with headers
text/plain 528b
Show quoted text
> I had a go at fixing this, and ended up with the patch below. But I > don't know how much of the problem this solves. It seems to work for > the few things I've tried it with, but it would need some proper > testing. Right now though I'll probably just rearrange my table and > column names.
Thanks. The 'proper' place for this is probably Class::DBI::Column, and having all the places that construct SQL call a method on that to get the correctly quoted version, but I'll play around with this and see where I get. Tony


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.