| Subject: | GROUP BY doesn't group on multiple columns |
The GROUP BY implementation is incomplete: while multiple columns are
accepted, SQL::Statement silently ignores all but the last named column.
In other words,
GROUP BY foo, bar, baz
is treated exactly the same as
GROUP BY baz
There's a test in the test suite that claims to be for 'GROUP BY several
columns', but unfortunately the test data has the property that
including or excluding the first column does not affect the grouping ...
so the test fails to fail. :)
I attach a little patch that fixes the test by providing a better set of
data.
Separately, I also attach a quick-and-dirty patch for the bug itself.
I've been using this myself for a couple of years, but you probably
won't want to apply it "as is"; it uses join() to create hash keys, with
all the problems that implies.
| Subject: | implement-multicolumn-group-by.patch |
diff -aur SQL-Statement-1.16_04/lib/SQL/Statement.pm SQL-Statement-1.16_04-patched/lib/SQL/Statement.pm
--- SQL-Statement-1.16_04/lib/SQL/Statement.pm 2009-01-04 23:01:16.000000000 +0000
+++ SQL-Statement-1.16_04-patched/lib/SQL/Statement.pm 2009-01-09 23:16:34.000000000 +0000
@@ -1445,7 +1445,7 @@
{
push @$set_cols, $_->{name};
}
- my ( $keycol, $keynum );
+ my @keycols = ();
for my $i ( 0 .. $numcols - 1 )
{
my $arg = $self->{"set_function"}->[$i]->{"arg"};
@@ -1456,8 +1456,7 @@
$arg = $set_cols->[$i];
# $arg =$columns_requested[$i];
- $keycol = $self->{"set_function"}->[$i]->{"name"};
- $keynum = $colnum{ uc $arg };
+ push @keycols, $colnum{ uc $arg };
}
$self->{"set_function"}->[$i]->{"sel_col_num"} = $colnum{ uc $arg };
}
@@ -1471,7 +1470,7 @@
# $keynum=$i if uc $display_cols->[$i]->{name} eq uc $keyfield;
#printf "%s.%s,%s\n",$i,$display_cols->[$i]->{name},$keyfield;
# }
- my $g = SQL::Statement::Group->new( $keynum, $display_cols, $rows );
+ my $g = SQL::Statement::Group->new( \@keycols, $display_cols, $rows );
$rows = $g->calc;
my $x = [ map { $_->{name} } @$display_cols ];
$self->{NAME} = [ map { $_->{name} } @$display_cols ];
@@ -2552,9 +2551,9 @@
sub new
{
my $class = shift;
- my ( $keycol, $display_cols, $ary ) = @_;
+ my ( $keycols, $display_cols, $ary ) = @_;
my $self = {
- keycol => $keycol,
+ keycols => $keycols,
display_cols => $display_cols,
records => $ary,
};
@@ -2634,15 +2633,16 @@
sub ary2hash
{
- my $self = shift;
- my $ary = shift;
- my $keycolnum = $self->{keycol} || 0;
+ my $self = shift;
+ my $ary = shift;
+ my @keycolnums = @{ $self->{keycols} || [0] };
my $hash;
my @keys;
my %is_key;
for my $row (@$ary)
{
- my $key = $row->[$keycolnum];
+ # This may fail if data contains \x01.
+ my $key = join "\x01", map { $row->[$_] } @keycolnums;
#die "@$row" unless defined $key;
push @{ $hash->{$key} }, $row;
| Subject: | fix-broken-test.patch |
diff -aur SQL-Statement-1.16_04/t/06group.t SQL-Statement-1.16_04-patched/t/06group.t
--- SQL-Statement-1.16_04/t/06group.t 2009-01-04 19:52:14.000000000 +0000
+++ SQL-Statement-1.16_04-patched/t/06group.t 2009-01-09 22:39:13.000000000 +0000
@@ -21,14 +21,14 @@
ok('2700~1000^' eq query2str($sth),'AGGREGATE FUNCTIONS WITHOUT GROUP BY');
$sth=$dbh->prepare("
- SELECT region,SUM(sales), MAX(sales) FROM biz GROUP BY region
+ SELECT class,SUM(sales), MAX(sales) FROM biz GROUP BY class
");
-ok('West~2000~1000^East~700~700^' eq query2str($sth),'GROUP BY one column');
+ok('Car~2000~1000^Truck~700~400^' eq query2str($sth),'GROUP BY one column');
$sth=$dbh->prepare("
- SELECT region,store,SUM(sales), MAX(sales) FROM biz GROUP BY region,store
+ SELECT color,class,SUM(sales), MAX(sales) FROM biz GROUP BY color,class
");
-ok('West~Los Angeles~1500~1000^West~San Diego~500~500^East~Boston~700~700^'
+ok('White~Car~1000~1000^Blue~Car~500~500^White~Truck~700~400^Red~Car~500~500^'
eq query2str($sth),'GROUP BY several columns');
sub query2str {
@@ -44,8 +44,9 @@
return $str;
}
__END__
-CREATE TEMP TABLE biz (region TEXT, store TEXT, sales INTEGER)
-INSERT INTO biz VALUES ('West','Los Angeles',1000 )
-INSERT INTO biz VALUES ('West','San Diego' ,500 )
-INSERT INTO biz VALUES ('West','Los Angeles',500 )
-INSERT INTO biz VALUES ('East','Boston' ,700 )
+CREATE TEMP TABLE biz (class TEXT, color TEXT, sales INTEGER)
+INSERT INTO biz VALUES ('Car' ,'White',1000)
+INSERT INTO biz VALUES ('Car' ,'Blue' ,500 )
+INSERT INTO biz VALUES ('Truck','White',400 )
+INSERT INTO biz VALUES ('Car' ,'Red' ,500 )
+INSERT INTO biz VALUES ('Truck','White',300 )