Make join tests behave

[dbsrgits/SQL-Abstract-2.0-ish.git] / lib / SQL / Abstract / AST / v1.pm

diff --git a/lib/SQL/Abstract/AST/v1.pm b/lib/SQL/Abstract/AST/v1.pm
index 3de8b66..a8860b5 100644 (file)
--- a/lib/SQL/Abstract/AST/v1.pm
+++ b/lib/SQL/Abstract/AST/v1.pm
@@ -32,25 +32,29 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
   }
 
   method _select(HashAST $ast) {
-    # Default to requiring columns and from
-    # Once TCs give better errors, make this a SelectAST type
-    for (qw/columns from/) {
-      confess "$_ key is required (and must be an AST) to select"
-        unless is_ArrayAST($ast->{$_});
+    # Default to requiring columns and from.
+    # DB specific ones (i.e. mysql/Pg) can not require the FROM part with a bit
+    # of refactoring
+    
+    for (qw/columns tablespec/) {
+      confess "'$_' is required in select AST with " . dump ($ast)
+        unless exists $ast->{$_};
     }
    
     # Check that columns is a -list
-    confess "columns key should be a -list AST, not " . $ast->{columns}[0]
-      unless $ast->{columns}[0] eq '-list';
+    confess "'columns' should be an array ref, not " . dump($ast->{columns})
+      unless is_ArrayRef($ast->{columns});
+
+    my $cols = $self->_list({-type => 'list', args => $ast->{columns} });
 
     my @output = (
-      "SELECT", 
-      $self->dispatch($ast->{columns}),
-      "FROM",
-      $self->dispatch($ast->{from})
+      SELECT => $cols
     );
 
-    for (qw/join/) {
+    push @output, FROM => $self->dispatch($ast->{tablespec})
+      if exists $ast->{tablespec};
+
+    for (qw//) {
       if (exists $ast->{$_}) {
         my $sub_ast = $ast->{$_};
         $sub_ast->{-type} = "$_" if is_HashRef($sub_ast);
@@ -64,10 +68,24 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
     return join(' ', @output);
   }
 
-  method _where(ArrayAST $ast) {
-    my (undef, @clauses) = @$ast;
+  method _join(HashRef $ast) {
+
+    # TODO: Validate join type
+    my $type = $ast->{join_type} || "";
   
-    return 'WHERE ' . $self->_recurse_where(\@clauses);
+    my @output = $self->dispatch($ast->{lhs});
+
+    push @output, uc $type if $type;
+    push @output, "JOIN", $self->dispatch($ast->{rhs});
+
+    push @output, 
+        exists $ast->{on}
+      ? ('ON', '(' . $self->_expr( $ast->{on} ) . ')' )
+      : ('USING', '(' .$self->dispatch($ast->{using} || croak "No 'on' or 'join' clause passed to -join").
+                  ')' );
+
+    return join(" ", @output);
+      
   }
 
   method _order_by(AST $ast) {
@@ -87,7 +105,7 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
     return "ORDER BY " . join(", ", @output);
   }
 
-  method _name(AST $ast) {
+  method _name(HashAST $ast) {
     my @names = @{$ast->{args}};
 
     my $sep = $self->name_separator;
@@ -112,18 +130,6 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
     return $ret;
   }
 
-  method _join(HashRef $ast) {
-  
-    my $output = 'JOIN ' . $self->dispatch($ast->{tablespec});
-
-    $output .= exists $ast->{on}
-             ? ' ON (' . $self->_recurse_where( $ast->{on} )
-             : ' USING (' .$self->dispatch($ast->{using} || croak "No 'on' or 'join' clause passed to -join");
-
-    $output .= ")";
-    return $output;
-      
-  }
 
   method _list(AST $ast) {
     my @items = @{$ast->{args}};
@@ -133,6 +139,7 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
       map { $self->dispatch($_) } @items);
   }
 
+  # TODO: I think i want to parameterized AST type to get better validation
   method _alias(AST $ast) {
     
     # TODO: Maybe we want qq{ AS "$as"} here