turn array references passed to group_by into a list of fields

mojolicious · Jan 27, 2018 · 94cf9eb · 94cf9eb
1 parent ca501cb
commit 94cf9eb
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 7 deletions.
diff --git a/lib/Mojo/Pg/Database.pm b/lib/Mojo/Pg/Database.pm
@@ -510,7 +510,7 @@ As well as some PostgreSQL specific extensions added by L<SQL::Abstract::Pg>.
   $db->select('some_table', '*', undef, {limit => 10, offset => 20});
 
   # "select * from some_table where foo = 23 group by foo, bar"
-  $db->select('some_table', '*', {foo => 23}, {group_by => \'foo, bar'});
+  $db->select('some_table', '*', {foo => 23}, {group_by => ['foo', 'bar']});
 
   # "select * from some_table where id = 1 for update skip locked"
   $db->select('some_table', '*', {id => 1}, {for => \'update skip locked'});

diff --git a/lib/SQL/Abstract/Pg.pm b/lib/SQL/Abstract/Pg.pm
@@ -51,11 +51,16 @@ sub _pg_parse {
   my $sql = '';
   my @bind;
   if (defined(my $group = $options->{group_by})) {
+    my $group_sql;
     $self->_SWITCH_refkind(
       $group => {
-        SCALARREF => sub { $sql .= $self->_sqlcase(' group by ') . $$group }
+        SCALARREF => sub { $group_sql = $$group },
+        ARRAYREF => sub {
+          $group_sql = join ', ', map { $self->_quote($_) } @$group;
+        }
       }
     );
+    $sql .= $self->_sqlcase(' group by ') . $group_sql;
   }
 
   # ORDER BY
@@ -76,11 +81,13 @@ sub _pg_parse {
 
   # FOR
   if (defined(my $for = $options->{for})) {
+    my $for_sql;
     $self->_SWITCH_refkind(
       $for => {
-        SCALARREF => sub { $sql .= $self->_sqlcase(' for ') . $$for }
+        SCALARREF => sub { $for_sql .= $$for }
       }
     );
+    $sql .= $self->_sqlcase(' for ') . $for_sql;
   }
 
   return $sql, @bind;
@@ -156,8 +163,11 @@ with C<LIMIT> and C<OFFSET> clauses.
 =head2 GROUP BY
 
 The C<group_by> option can be used to generate C<SELECT> queries with
-C<GROUP BY> clauses. So far only scalar references to pass literal SQL are
-supported.
+C<GROUP BY> clauses. So far only array references to pass a list of fields and
+scalar references to pass literal SQL are supported.
+
+  # "select * from some_table group by foo, bar"
+  $abstract->select('some_table', undef, undef, {group_by => ['foo', 'bar']});
 
   # "select * from some_table group by foo, bar"
   $abstract->select('some_table', undef, undef, {group_by => \'foo, bar'});

diff --git a/t/sql.t b/t/sql.t
@@ -61,10 +61,12 @@ is_deeply \@sql, ['SELECT * FROM "foo" LIMIT ? OFFSET ?', 10, 5], 'right query';
 # GROUP BY
 @sql = $abstract->select('foo', '*', undef, {group_by => \'bar, baz'});
 is_deeply \@sql, ['SELECT * FROM "foo" GROUP BY bar, baz'], 'right query';
+@sql = $abstract->select('foo', '*', undef, {group_by => ['bar', 'baz']});
+is_deeply \@sql, ['SELECT * FROM "foo" GROUP BY "bar", "baz"'], 'right query';
 
 # GROUP BY (unsupported value)
-eval { $abstract->select('foo', '*', undef, {group_by => []}) };
-like $@, qr/ARRAYREF/, 'right error';
+eval { $abstract->select('foo', '*', undef, {group_by => {}}) };
+like $@, qr/HASHREF/, 'right error';
 
 # FOR
 @sql = $abstract->select('foo', '*', undef, {for => \'update skip locked'});