Skip Menu |
 

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

Report information
The Basics
Id: 32133
Status: new
Priority: 0/
Queue: Class-DBI-Plugin-Backtickify

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

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



Subject: test file; patch: backtickify_arg(), transform_sql() fixes
Download (untitled) / with headers
text/plain 1.8k
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
Download backtickify.t
text/x-perl 2.8k
#!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
Download backtickify.patch
text/x-diff 3.8k
*** ../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 $@;


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.