| Subject: | Join syntax is case-sensitive, and common columns in natural joins are "ambiguous" [patch] |
The attached patch (against current SVN) attempts to fix the following issues:
* The parsing for join keywords in SQL::Parser::EXPLICIT_JOIN relied on their being upper-case, which is a bad thing to rely on.
* It shouldn't be necessary to specify which table the common column from a natural join comes from--in fact, in a natural join, there are _no_ ambiguous columns, by definition. Which made that an easy fix.
* Columns specified in a "using" clause in some systems (OK, fine, I mean Oracle) need not (indeed, may not) have a table identifier if they are also specified in the "select" clause. This makes them "ambiguous" to SQL::Statement, even though again, they're not actually ambiguous (also, it makes them a hassle for outer joins).
This last point is probably worth an example. Here's the currently required syntax:
SELECT A.foo,A.common,B.bar from A join B using (common)
This statement (or its equivalent) will cause Oracle to say this...
ORA-25154: column part of USING clause cannot have qualifier
... and to force you to use this syntax:
SELECT A.foo,common,B.bar from A join B using (common)
The patch makes both options equally valid. Ideally, this behavior would probably be controlled by an option, such that either Oracle-like or MySQL-like behavior would be supported, but not both at once, but that was beyond my current understanding of the code, so I didn't try it.
Arguably, this state of excessive permissiveness would just make things worse, but that part of the patch is very short (one statement), and easily excised if need be.
diff -ur SQL-Statement-1.15.orig/lib/SQL/Parser.pm SQL-Statement-1.15/lib/SQL/Parser.pm
--- SQL-Statement-1.15.orig/lib/SQL/Parser.pm 2005-04-18 20:41:09.000000000 -0400
+++ SQL-Statement-1.15/lib/SQL/Parser.pm 2005-11-10 16:33:13.000000000 -0500
@@ -467,45 +467,45 @@
my $remainder = shift;
return undef unless $remainder;
my($tableA,$tableB,$keycols,$jtype,$natural);
- if ($remainder =~ /^(.+?) (NATURAL|INNER|LEFT|RIGHT|FULL|UNION|JOIN)(.+)$/s){
+ if ($remainder =~ /^(.+?) (NATURAL|INNER|LEFT|RIGHT|FULL|UNION|JOIN)(.+)$/is){
$tableA = $1;
$remainder = $2.$3;
}
else {
($tableA,$remainder) = $remainder =~ /^(\S+) (.*)/;
}
- if ( $remainder =~ /^NATURAL (.+)/) {
+ if ( $remainder =~ /^NATURAL (.+)/i) {
$self->{"struct"}->{join}->{"clause"} = 'NATURAL';
$natural++;
$remainder = $1;
}
if ( $remainder =~
- /^(INNER|LEFT|RIGHT|FULL|UNION) JOIN (.+)/
+ /^(INNER|LEFT|RIGHT|FULL|UNION) JOIN (.+)/i
) {
- $jtype = $self->{"struct"}->{join}->{"clause"} = $1;
+ $jtype = $self->{"struct"}->{join}->{"clause"} = uc $1;
$remainder = $2;
- $jtype = "$jtype OUTER" if $jtype !~ /INNER|UNION/;
+ $jtype = "$jtype OUTER" if $jtype !~ /INNER|UNION/i;
}
if ( $remainder =~
- /^(LEFT|RIGHT|FULL) OUTER JOIN (.+)/
+ /^(LEFT|RIGHT|FULL) OUTER JOIN (.+)/i
) {
- $jtype = $self->{"struct"}->{join}->{"clause"} = $1 . " OUTER";
+ $jtype = $self->{"struct"}->{join}->{"clause"} = uc($1) . " OUTER";
$remainder = $2;
}
- if ( $remainder =~ /^JOIN (.+)/) {
+ if ( $remainder =~ /^JOIN (.+)/i) {
$jtype = 'INNER';
$self->{"struct"}->{join}->{"clause"} = 'DEFAULT INNER';
$remainder = $1;
}
if ( $self->{"struct"}->{join} ) {
- if ( $remainder && $remainder =~ /^(.+?) USING \(([^\)]+)\)(.*)/) {
+ if ( $remainder && $remainder =~ /^(.+?) USING \(([^\)]+)\)(.*)/i) {
$self->{"struct"}->{join}->{"clause"} = 'USING';
$tableB = $1;
my $keycolstr = $2;
$remainder = $3;
@$keycols = split /,/,$keycolstr;
}
- if ( $remainder && $remainder =~ /^(.+?) ON (.+)/) {
+ if ( $remainder && $remainder =~ /^(.+?) ON (.+)/i) {
$self->{"struct"}->{join}->{"clause"} = 'ON';
$tableB = $1;
my $keycolstr = $2;
diff -ur SQL-Statement-1.15.orig/lib/SQL/Statement.pm SQL-Statement-1.15/lib/SQL/Statement.pm
--- SQL-Statement-1.15.orig/lib/SQL/Statement.pm 2005-09-26 14:49:35.000000000 -0400
+++ SQL-Statement-1.15/lib/SQL/Statement.pm 2005-11-10 16:10:52.000000000 -0500
@@ -1574,6 +1574,18 @@
@duplicates = map {s/[^.]*\.(.*)/$1/; $_} @$all_cols;
@duplicates = grep($is_member{$_}++, @duplicates);
%is_duplicate = map { $_=>1} @duplicates;
+ if (my $join = $self->{join} ) {
+ if ( $join->{type} =~ /NATURAL/i ) {
+ %is_duplicate = ();
+ }
+ # the following should be probably conditioned on an option,
+ # but I don't know which --BW
+ elsif ( 'USING' eq $join->{clause} ) {
+ my @keys = @{$join->{keycols}};
+ delete @is_duplicate{@keys};
+ @short_exists{@keys} = (($self->tables)[0]->{name}) x @keys;
+ }
+ }
my $is_fully;
my $i=-1;
my $num_tables = $self->tables;
diff -ur SQL-Statement-1.15.orig/t/08join.t SQL-Statement-1.15/t/08join.t
--- SQL-Statement-1.15.orig/t/08join.t 2005-09-26 14:55:36.000000000 -0400
+++ SQL-Statement-1.15/t/08join.t 2005-11-10 16:27:04.000000000 -0500
@@ -8,55 +8,102 @@
plan skip_all => "No DBD::File available";
}
else {
- plan tests => 8;
+ plan tests => 14;
}
use SQL::Statement; printf "SQL::Statement v.%s\n", $SQL::Statement::VERSION;
use vars qw($dbh $sth $DEBUG);
$DEBUG = 0;
-$dbh = DBI->connect('dbi:File(RaiseError=1):');
+$dbh = DBI->connect('dbi:File(PrintError=1):');
$dbh->do($_) for <DATA>;
-$sth = $dbh->prepare("SELECT pname,sname FROM Prof NATURAL JOIN Subject");
-ok( 'Sue~Chem^Bob~Bio^Bob~Math^'
- eq query2str($sth),'NATURAL JOIN - with named columns in select list');
+queryresult_is(
+ "SELECT pname,sname FROM Prof NATURAL JOIN Subject",
+ 'Sue~Chem^Bob~Bio^Bob~Math^',
+ 'NATURAL JOIN - with named columns in select list'
+);
+
+queryresult_is("SELECT * FROM Prof NATURAL JOIN Subject",
+ '1~Sue~Chem^2~Bob~Bio^2~Bob~Math^',
+ 'NATURAL JOIN - with select list = *'
+);
-
-$sth = $dbh->prepare("SELECT * FROM Prof NATURAL JOIN Subject");
-ok( '1~Sue~Chem^2~Bob~Bio^2~Bob~Math^'
- eq query2str($sth),'NATURAL JOIN - with select list = *');
-
-$sth = $dbh->prepare("
+queryresult_is("
SELECT UPPER(pname)AS P,Prof.pid,pname,sname FROM Prof NATURAL JOIN Subject
-");
-ok( 'SUE~1~Sue~Chem^BOB~2~Bob~Bio^BOB~2~Bob~Math^'
- eq query2str($sth),'NATURAL JOIN - with computed columns');
-
-$sth = $dbh->prepare("SELECT * FROM Prof LEFT JOIN Subject USING(pid)");
-ok( '1~Sue~Chem^2~Bob~Bio^2~Bob~Math^3~Tom~undef^'
- eq query2str($sth),'LEFT JOIN');
-
-$sth = $dbh->prepare("SELECT * FROM Prof RIGHT JOIN Subject USING(pid)");
-ok( '1~Chem~Sue^2~Bio~Bob^2~Math~Bob^4~English~undef^'
- eq query2str($sth),'RIGHT JOIN');
-
-$sth = $dbh->prepare("SELECT * FROM Prof FULL JOIN Subject USING(pid)");
-ok( '1~Sue~Chem^2~Bob~Bio^2~Bob~Math^3~Tom~undef^4~undef~English^'
- eq query2str($sth),'FULL JOIN');
+",
+ 'SUE~1~Sue~Chem^BOB~2~Bob~Bio^BOB~2~Bob~Math^',
+ 'NATURAL JOIN - with computed columns'
+);
+
+queryresult_is(
+ "SELECT UPPER(pname)AS P,pid,pname,sname FROM Prof NATURAL JOIN Subject",
+ 'SUE~1~Sue~Chem^BOB~2~Bob~Bio^BOB~2~Bob~Math^',
+ 'NATURAL JOIN - with no specifier on join column'
+);
+
+queryresult_is(
+ "SELECT UPPER(pname)AS P,pid,pname,sname FROM Prof JOIN Subject using (pid)",
+ 'SUE~1~Sue~Chem^BOB~2~Bob~Bio^BOB~2~Bob~Math^',
+ 'INNER JOIN - with no specifier on join column'
+);
+
+queryresult_is("SELECT * FROM Prof LEFT JOIN Subject USING(pid)",
+ '1~Sue~Chem^2~Bob~Bio^2~Bob~Math^3~Tom~undef^',
+ 'LEFT JOIN'
+);
+
+queryresult_is("SELECT pid,pname,sname FROM Prof LEFT JOIN Subject USING(pid)",
+ '1~Sue~Chem^2~Bob~Bio^2~Bob~Math^3~Tom~undef^',
+ 'LEFT JOIN - enumerated columns'
+);
+
+queryresult_is("SELECT subject.pid,pname,sname FROM Prof LEFT JOIN Subject USING(pid)",
+ '1~Sue~Chem^2~Bob~Bio^2~Bob~Math^undef~Tom~undef^',
+ 'LEFT JOIN - perversely intentionally mis-enumerated columns'
+);
+
+queryresult_is("select subject.pid,pname,sname from prof left join subject using(pid)",
+ '1~Sue~Chem^2~Bob~Bio^2~Bob~Math^undef~Tom~undef^',
+ 'LEFT JOIN - lower case keywords'
+);
+
+queryresult_is("SELECT * FROM Prof RIGHT JOIN Subject USING(pid)",
+ '1~Chem~Sue^2~Bio~Bob^2~Math~Bob^4~English~undef^',
+ 'RIGHT JOIN'
+);
+
+queryresult_is("SELECT pid,sname,pname FROM Prof RIGHT JOIN Subject USING(pid)",
+ '1~Chem~Sue^2~Bio~Bob^2~Math~Bob^4~English~undef^',
+ 'RIGHT JOIN - enumerated columns'
+);
+
+queryresult_is("SELECT * FROM Prof FULL JOIN Subject USING(pid)",
+ '1~Sue~Chem^2~Bob~Bio^2~Bob~Math^3~Tom~undef^4~undef~English^',
+ 'FULL JOIN'
+);
-$sth = $dbh->prepare("
+queryresult_is("
SELECT * FROM Prof AS P,Subject AS S WHERE P.pid=S.pid
-");
-ok( '1~Sue~1~Chem^2~Bob~2~Bio^2~Bob~2~Math^'
- eq query2str($sth),'IMPLICIT JOIN - two tables');
+",
+ '1~Sue~1~Chem^2~Bob~2~Bio^2~Bob~2~Math^',
+ 'IMPLICIT JOIN - two tables'
+);
-$sth = $dbh->prepare("
+queryresult_is("
SELECT *
FROM Prof AS P,Subject AS S,Room AS R
WHERE P.pid=S.pid
AND P.pid=R.pid
-");
-ok( '1~Sue~1~Chem~1~1C^2~Bob~2~Bio~2~2B^2~Bob~2~Math~2~2B^'
- eq query2str($sth),'IMPLICIT JOIN - three tables');
+",
+ '1~Sue~1~Chem~1~1C^2~Bob~2~Bio~2~2B^2~Bob~2~Math~2~2B^',
+ 'IMPLICIT JOIN - three tables'
+);
+
+sub queryresult_is {
+ my ($query,$expected,$desc) = @_;
+ my $sth = $dbh->prepare($query);
+ my $result = query2str($sth);
+ is($result,$expected,$desc);
+}
sub query2str {
my($sth)=@_;