| Subject: | test file; patch: backtickify_arg(), transform_sql() fixes |
Everything is in reference to Class-DBI-Plugin-Backtickify-0.02,
i386-linux, v5.6.1.
Test file increases coverage from ~5% to > 60%
This module was a life-saver while trying to get some Class::DBI class
setup for an obviously ill-designed .mdb file!
The problem i hit (and I hit this real -- have a live db that actually
has this unfortunate naming) is when there's a column "foo bar" and a
column "foo" ... Depending on the order, that result in "`foo` bar" or
"``foo` bar`".
My first fix was this:
my $backtickify_arg = sub {
$_[0] =~ s/\b(?<!`)$_(?!`)\b/`$_`/g for
sort { length "$b" <=> length "$a" }
$self->all_columns
};
Which includes two fixes -- first, the negative look-ahead/look-behinds
make sure something doesn't get quoted twice. Second, it processes the
columns in order of longest to shortest. Now, w/the "foo bar" example,
it will go: "foo bar" --(check "foo bar" column)--> finds it ==> "`foo
bar`" --(check for "foo" column)--> no match.
I later changed the backtickify_arg() to something more complicated
(still includes the above functionality), which is in the patch file.
Because the above still doesn't cover the case of columns of "foo bar
stuff" and "bar". That will become "`foo `bar` stuff`". This
(recursive, btw) backtickify_arg() now handles that case.
The patch file also reflects the changes to the __FOO__-matching regexps
that are suggested for Class::DBI in RT#32115 -- the tests are tagged at
TODO in the test file.
The patch file also includes changes to the eval()'ing that
transform_sql() does. This was prompted by the test case of
__TABLE)foo__ blowing up, because (i think) that unmatched paren messes
with the q() used to construct the string to eval. Instead of
constructing the string to eval, now it simply does:
my $return = eval { $super->transform_sql( $sql, @args ) };
| Subject: | backtickify.t |
#!perl
use strict;
use warnings;
use Test::More tests => 59;
$|=1;
package Test;
use base qw/Class::DBI/;
__PACKAGE__->connection('DBI:Mock:', '', '');
__PACKAGE__->table('table name');
our @COLS = (
# primary:
'some id',
# essential:
'foo bar stuff',
'foo bar',
'foo',
'bar',
'more',
'more stuff',
'things',
'other things',
);
__PACKAGE__->columns( Primary => $COLS[0]);
__PACKAGE__->columns( Essential => @COLS[1..$#COLS] );
# hack so that we get back the columns in the order specified above.
# otherwise, Class::DBI::ColumnGrouper just returns values(%somehash)
# and order is not consistent/predictable
# known order is necessary to accurately execute test cases
sub all_columns {
my $self = shift;
my @cols = $self->SUPER::all_columns;
my %index;
@index{ @COLS } = 0 .. $#COLS;
return sort { $index{$a} <=> $index{$b} } @cols;
}
package Blah;
use base qw/Class::DBI/;
__PACKAGE__->table('blah table name');
package main;
use_ok( 'Class::DBI::Plugin::Backtickify' );
my $class = 'Test';
foreach my $test (
map {
my $expected = "`$_`";
(
[ $_, "`$_`" ], # as-is
[ "`$_`", "`$_`" ], # pre-quoted
[ "qwe $_ asd", "qwe `$_` asd" ], # in some context
[ "qwe `$_` asd", "qwe `$_` asd" ], # pre-quoted in some context
)
}
'some id',
'foo bar',
'foo',
'bar',
'more',
'more stuff',
'things',
'other things',
'foo bar stuff',
){
#next unless $test->[0] =~ /other/;
#warn "=========== " . $test->[0];
my $s = Class::DBI::Plugin::Backtickify::transform_sql( $class, '%s', $test->[0] );
is( $s, $test->[1], "arg: ".$test->[0] );
}
foreach my $test (
[ '__TABLE__', '`table name`' ],
[ '__TABLE(Test)__', '`table name`' ],
[ '__IDENTIFIER__', '`some id`=?' ],
[ '__ESSENTIAL__', '`some id`, `foo bar stuff`, `foo bar`, `foo`, `bar`, `more`, `more stuff`, `things`, `other things`' ],
[ '__FOO__', '__FOO__' ],
[ '__TABLE(Blah)__', '`blah table name`'],
[ 'blarg', 'blarg'],
[ 'blarg(open paren', 'blarg(open paren'],
[ 'blarg)close paren', 'blarg)close paren'],
[ '__ESSENTIAL(Blah)__', '`Blah`.`some id`, `Blah`.`foo bar stuff`, `Blah`.`foo bar`, `Blah`.`foo`, `Blah`.`bar`, `Blah`.`more`, `Blah`.`more stuff`, `Blah`.`things`, `Blah`.`other things`'],
){
my $s = eval { Class::DBI::Plugin::Backtickify::transform_sql( $class, $test->[0], () ) };
is( $s, $test->[1], "sql: ".$test->[0] );
}
TODO: {
local $TODO = 'Class-DBI RT#32115';
foreach my $test (
map {
my $expected = "`$_`";
(
# all should come through verbatim
[ $_, "$_" ], # as-is
[ "qwe $_ asd", "qwe $_ asd" ], # in some context
)
}
'__TABLE(Blah__',
'__TABLEBlah)__',
'__TABLEBlah__',
'__TABLE()__',
'__JOIN()__',
'__ESSENTIAL()__',
){
my $s = eval { Class::DBI::Plugin::Backtickify::transform_sql( $class, $test->[0], () ) };
is( $s, $test->[1], "sql: ".$test->[0] );
}
}
exit;
| Subject: | backtickify.patch |
*** ../Class-DBI-Plugin-Backtickify-0.02/lib/Class/DBI/Plugin/Backtickify.pm Tue May 24 10:23:48 2005
--- lib/Class/DBI/Plugin/Backtickify.pm Mon Jan 7 07:06:45 2008
***************
*** 59,64 ****
--- 59,84 ----
=cut
+ sub backtickify_arg {
+ my $self = shift;
+ return $_[0] if $_[0] =~ /^`[^`]*`$/; # return if already quoted
+ my @cols = $_[1]
+ ? @{$_[1]} # use what's given us (in the recursion cases)
+ # or (the initial case) use all cols, sorted longest to shortest
+ # This is necessary so that 'foo bar' gets processed before 'foo',
+ # so that if you have "foo bar" it doesn't become "`foo` bar"
+ : sort { length $b <=> length $a } map { "$_" } $self->all_columns
+ ;
+ return $_[0] unless @cols;
+ my $c = shift @cols; # process first col
+ $_[0] =~ s/\b(?<!`)$c(?!`)\b/`$c`/g; # quote it where it's currently unquoted
+ # Recurse on all the pieces w/the remaining columns to process.
+ # Note the the quoted ones will just return right way.
+ my @s = map { backtickify_arg($self,$_,\@cols) } split /(`$c`)/, $_[0];
+ $_[0] = join '', @s;
+ return $_[0];
+ }
+
sub transform_sql
{
my ( $self, $sql, @args ) = @_;
***************
*** 67,79 ****
# Each entry in @args is a SQL fragment. This will bugger with fragments that
# contain strings that match column names but are not supposed to be column names.
! my $backtickify_arg = sub { $_[0] =~ s/\b$_\b/`$_`/g for $self->all_columns };
! $backtickify_arg->( $_ ) for @args;
# -------------------
my %cmap;
my $expand_table = sub {
! my ($class, $alias) = split /=/, shift, 2;
my $table = $class ? $class->table : $self->table;
$cmap{ $alias || $table } = $class || ref $self || $self;
($alias ||= "") &&= " AS `$alias`";
--- 87,99 ----
# Each entry in @args is a SQL fragment. This will bugger with fragments that
# contain strings that match column names but are not supposed to be column names.
! backtickify_arg( $self, $_ ) for @args;
# -------------------
my %cmap;
my $expand_table = sub {
! my $s = shift;
! my ($class, $alias) = split /=/, defined($s)?$s:'', 2;
my $table = $class ? $class->table : $self->table;
$cmap{ $alias || $table } = $class || ref $self || $self;
($alias ||= "") &&= " AS `$alias`";
***************
*** 111,120 ****
};
# -------------------
! $sql =~ s/__TABLE\(?(.*?)\)?__/$expand_table->($1)/eg;
! $sql =~ s/__JOIN\((.*?)\)__/$expand_join->($1)/eg;
$sql =~ s/__ESSENTIAL__/join ", ", map { "`$_`" } $self->_essential/eg;
! $sql =~ s/__ESSENTIAL\((.*?)\)__/join ", ", map { "`$1`.`$_`" } $self->_essential/eg;
if ( $sql =~ /__IDENTIFIER__/ )
{
--- 131,140 ----
};
# -------------------
! $sql =~ s/__TABLE(?:\((.+?)\))?__/$expand_table->($1)/eg;
! $sql =~ s/__JOIN\((.+?)\)__/$expand_join->($1)/eg;
$sql =~ s/__ESSENTIAL__/join ", ", map { "`$_`" } $self->_essential/eg;
! $sql =~ s/__ESSENTIAL\((.+?)\)__/join ", ", map { "`$1`.`$_`" } $self->_essential/eg;
if ( $sql =~ /__IDENTIFIER__/ )
{
***************
*** 124,135 ****
# nasty hack
my $super = ( Class::ISA::super_path( ref( $self ) || $self ) )[0];
!
! my $eval = '{ package %s; $self->SUPER::transform_sql( q(%s), ';
! $eval .= 'q(%s), ' for @args;
! $eval .= ') }';
!
! my $return = eval sprintf $eval, $super, $sql, @args;
die $@ if $@;
--- 144,150 ----
# nasty hack
my $super = ( Class::ISA::super_path( ref( $self ) || $self ) )[0];
! my $return = eval { $super->transform_sql( $sql, @args ) };
die $@ if $@;