From: Ash Berlin <ash_github@firemirror.com>
Date: Sat, 14 Mar 2009 17:17:02 +0000 (+0000)
Subject: ORDER BY, HAVING, GROUP BY and WHERE clauses on select
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=924d940e086f06f27ba96103fb3aa8ecb9656def;p=dbsrgits%2FSQL-Abstract-2.0-ish.git

ORDER BY, HAVING, GROUP BY and WHERE clauses on select
---

diff --git a/lib/SQL/Abstract/AST/v1.pm b/lib/SQL/Abstract/AST/v1.pm
index d0f4522..ae2e05c 100644
--- a/lib/SQL/Abstract/AST/v1.pm
+++ b/lib/SQL/Abstract/AST/v1.pm
@@ -54,12 +54,21 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
     push @output, FROM => $self->dispatch($ast->{tablespec})
       if exists $ast->{tablespec};
 
-    for (qw/where having group_by/) {
+    if (exists $ast->{where}) {
+      my $sub_ast = $ast->{where};
+
+      confess "$_ option is not an AST: " . dump($sub_ast)
+        unless is_AST($sub_ast);
+
+      push @output, "WHERE", $self->_expr($sub_ast);
+    }
+
+    for (qw/group_by having order_by/) {
       if (exists $ast->{$_}) {
         my $sub_ast = $ast->{$_};
 
-        confess "$_ option is not an AST: " . dump($sub_ast)
-          unless is_AST($sub_ast);
+        confess "$_ option is not an AST or an ArrayRef: " . dump($sub_ast)
+          unless is_AST($sub_ast) || is_ArrayRef($sub_ast);;
 
         my $meth = "__$_";
         push @output, $self->$meth($sub_ast);
@@ -82,28 +91,27 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
     push @output, 
         exists $ast->{on}
       ? ('ON', '(' . $self->_expr( $ast->{on} ) . ')' )
-      : ('USING', '(' .$self->dispatch($ast->{using} || croak "No 'on' or 'join' clause passed to -join").
+      : ('USING', '(' .$self->dispatch($ast->{using} 
+                        || croak "No 'on' or 'uinsg' clause passed to join cluase: " .
+                                 dump($ast) 
+                        ) .
                   ')' );
 
     return join(" ", @output);
       
   }
 
-  method _order_by(AST $ast) {
-    my @clauses = @{$ast->{order_by}};
-  
-    my @output;
-   
-    for (@clauses) {
-      if (is_ArrayRef($_) && $_->[0] =~ /^-(asc|desc)$/) {
-        my $o = $1;
-        push @output, $self->dispatch($_->[1]) . " " . uc($o);
-        next;
-      }
-      push @output, $self->dispatch($_);
-    }
+  method _ordering(AST $ast) {
+ 
+    my $output = $self->_expr($ast->{expr});
+
+    $output .= " " . uc $1
+      if $ast->{direction} && 
+         ( $ast->{direction} =~ /^(asc|desc)$/i 
+           || confess "Unknown ordering direction " . dump($ast)
+         );
 
-    return "ORDER BY " . join(", ", @output);
+    return $output;
   }
 
   method _name(HashAST $ast) {
@@ -133,7 +141,9 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
 
 
   method _list(AST $ast) {
-    my @items = @{$ast->{args}};
+    my @items = is_ArrayRef($ast->{args})
+              ? @{$ast->{args}}
+              : $ast->{args};
 
     return join(
       $self->list_separator,
@@ -155,10 +165,19 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
   }
 
   # Not dispatchable to.
-  method __where(HashAST $ast) {
-    return "WHERE " . $self->_expr($ast);
+  method __having($args) {
+    return "HAVING " . $self->_list({-type => 'list', args => $args});
+  }
+
+  method __group_by($args) {
+    return "GROUP BY " . $self->_list({-type => 'list', args => $args});
   }
 
+  method __order_by($args) {
+    return "ORDER BY " . $self->_list({-type => 'list', args => $args});
+  }
+
+
   # Perhaps badly named. handles 'and' and 'or' clauses
   method _recurse_where(HashAST $ast) {
 
diff --git a/t/001_basic.t b/t/001_basic.t
index 728d948..849a982 100644
--- a/t/001_basic.t
+++ b/t/001_basic.t
@@ -1,10 +1,11 @@
 use strict;
 use warnings;
 
-use Test::More tests => 8;
+use Test::More tests => 9;
 use Test::Differences;
 
 use_ok('SQL::Abstract') or BAIL_OUT( "$@" );
+use_ok('SQL::Abstract::AST::v1') or BAIL_OUT( "$@" );
 
 my $sqla = SQL::Abstract->create(1);
 
diff --git a/t/201_select.t b/t/201_select.t
index d27a327..b20dc46 100644
--- a/t/201_select.t
+++ b/t/201_select.t
@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 
-use Test::More tests => 4;
+use Test::More tests => 5;
 use Test::Differences;
 
 use_ok('SQL::Abstract') or BAIL_OUT( "$@" );
@@ -72,3 +72,19 @@ is $sqla->dispatch(
 
 ), "SELECT me.* FROM foo AS me WHERE me.id = ?",
    "select with where";
+
+
+is $sqla->dispatch(
+  { -type => 'select',
+    tablespec => $foo_as_me,
+    columns => [
+      { -type => 'name', args => [qw/me id/] },
+      { -type => 'alias', ident => $me_foo_id, as => 'foo' },
+    ],
+    order_by => [
+      { -type => 'ordering', expr => { -type => 'name', args => [qw/me name/] }, direction => 'desc' },
+      $me_foo_id,
+    ]
+  }
+), "SELECT me.id, me.foo_id AS foo FROM foo AS me ORDER BY me.name DESC, me.foo_id",
+   "select clause with order by";
diff --git a/t/300_order_by.t b/t/300_order_by.t
deleted file mode 100644
index ee12886..0000000
--- a/t/300_order_by.t
+++ /dev/null
@@ -1,35 +0,0 @@
-use strict;
-use warnings;
-
-use Test::More tests => 4;
-use Test::Differences;
-
-use_ok('SQL::Abstract') or BAIL_OUT( "$@" );
-
-my $sqla = SQL::Abstract->create(1);
-
-
-is $sqla->dispatch(
-  { -type => 'order_by', order_by => [ { -type => name => args => [qw/me date/ ] } ] }
-), "ORDER BY me.date",
-   "order by";
-
-is $sqla->dispatch(
-  { -type => 'order_by',
-    order_by => [
-      { -type => name => args => [qw/me date/] },
-      { -type => name => args => [qw/me foobar/] },
-    ]
-  }
-), "ORDER BY me.date, me.foobar",
-   "order by";
-
-# Hrmmm how to mark this up.
-is $sqla->dispatch(
-  { -type => 'order_by', 
-    order_by => [
-      [ -desc => { -type => name => args => [qw/me date/ ] } ] 
-    ] 
-  }
-), "ORDER BY me.date DESC",
-   "order by desc";
diff --git a/t/300_ordering.t b/t/300_ordering.t
new file mode 100644
index 0000000..cf5456a
--- /dev/null
+++ b/t/300_ordering.t
@@ -0,0 +1,32 @@
+use strict;
+use warnings;
+
+use Test::More tests => 4;
+use Test::Differences;
+
+use_ok('SQL::Abstract') or BAIL_OUT( "$@" );
+
+my $sqla = SQL::Abstract->create(1);
+
+
+is $sqla->dispatch(
+  { -type => 'ordering', expr => { -type => name => args => [qw/me date/ ] } }
+), "me.date",
+   "basic ordering";
+
+is $sqla->dispatch(
+  { -type => 'ordering', 
+    expr => { -type => name => args => [qw/me date/] },
+    direction => 'DESC'
+  }
+), "me.date DESC",
+   "desc ordering";
+
+
+is $sqla->dispatch(
+  { -type => 'ordering', 
+    expr => { -type => name => args => [qw/me date/] },
+    direction => 'asc'
+  }
+), "me.date ASC",
+   "asc ordering";