LEFT JOIN + rework join tests
[dbsrgits/SQL-Abstract-2.0-ish.git] / lib / SQL / Abstract / AST / v1.pm
index 7aa1711..0ce6e9f 100644 (file)
@@ -9,44 +9,52 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
   use MooseX::Types::Moose qw/ArrayRef Str Int Ref HashRef/;
   use MooseX::AttributeHelpers;
   use SQL::Abstract::Types qw/AST ArrayAST HashAST/;
+  use Devel::PartialDump qw/dump/;
 
   clean;
 
   # set things that are valid in where clauses
-  override _build_where_dispatch_table {
+  override _build_expr_dispatch_table {
     return { 
       %{super()},
       in => $self->can('_in'),
       not_in => $self->can('_in'),
+      and => $self->can('_recurse_where'),
+      or => $self->can('_recurse_where'),
       map { +"$_" => $self->can("_$_") } qw/
         value
         name
         true
         false
+        expr
       /
     };
   }
 
   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);
@@ -60,10 +68,28 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
     return join(' ', @output);
   }
 
-  method _where(ArrayAST $ast) {
-    my (undef, @clauses) = @$ast;
+  method _join(HashRef $ast) {
+    confess "'args' option to join should be an array ref, not " . dump($ast->{args})
+      unless is_ArrayRef($ast->{args});
+
+    my ($from, $to) = @{ $ast->{args} };
+
+    # TODO: Validate join type
+    my $type = $ast->{join_type} || "";
   
-    return 'WHERE ' . $self->_recurse_where(\@clauses);
+    my @output = $self->dispatch($from);
+
+    push @output, uc $type if $type;
+    push @output, "JOIN", $self->dispatch($to);
+
+    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) {
@@ -83,7 +109,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;
@@ -108,18 +134,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}};
@@ -129,6 +143,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
@@ -142,32 +157,21 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
     return "?";
   }
 
-  method _recurse_where(ArrayRef $clauses) {
+  # Perhaps badly named. handles 'and' and 'or' clauses
+  method _recurse_where(HashAST $ast) {
 
-    my $OP = 'AND';
-    my $prio = $SQL::Abstract::PRIO{and};
-    my $first = $clauses->[0];
+    my $op = $ast->{op};
 
-    if (!ref $first) {
-      if ($first =~ /^-(and|or)$/) {
-        $OP = uc($1);
-        $prio = $SQL::Abstract::PRIO{$1};
-        shift @$clauses;
-      } else {
-        # If first is not a ref, and its not -and or -or, then $clauses
-        # contains just a single clause
-        $clauses = [ $clauses ];
-      }
-    }
+    my $OP = uc $op;
+    my $prio = $SQL::Abstract::PRIO{$op};
 
-    my $dispatch_table = $self->where_dispatch_table;
+    my $dispatch_table = $self->expr_dispatch_table;
 
     my @output;
-    foreach (@$clauses) {
-      croak "invalid component in where clause: $_" unless is_ArrayRef($_);
-      my $op = $_->[0];
+    foreach ( @{$ast->{args}} ) {
+      croak "invalid component in where clause: $_" unless is_HashAST($_);
 
-      if ($op =~ /^-(and|or)$/) {
+      if ($_->{-type} eq 'expr' && $_->{op} =~ /^(and|or)$/) {
         my $sub_prio = $SQL::Abstract::PRIO{$1}; 
 
         if ($sub_prio <= $prio) {
@@ -176,53 +180,50 @@ class SQL::Abstract::AST::v1 extends SQL::Abstract {
           push @output, '(' . $self->_recurse_where($_) . ')';
         }
       } else {
-        push @output, $self->_where_component($_);
+        push @output, $self->_expr($_);
       }
     }
 
     return join(" $OP ", @output);
   }
 
-  method _where_component(HashAST $ast) {
+  method _expr(HashAST $ast) {
     my $op = $ast->{-type};
 
-    if (my $code = $self->lookup_where_dispatch($op)) { 
+    $op = $ast->{op} if $op eq 'expr';
+
+    if (my $code = $self->lookup_expr_dispatch($op)) { 
       
       return $code->($self, $ast);
 
     }
-    croak "'$op' is not a valid clause in a where AST"
-      if $op =~ /^-/;
+    croak "'$op' is not a valid AST type in an expression with " . dump($ast)
+      if $ast->{-type} ne 'expr';
 
-    croak "'$op' is not a valid operator";
+    croak "'$op' is not a valid operator in an expression with " . dump($ast);
    
   }
 
-  method _expr(HashAST $ast) {
-    my $op = $ast->{op};
-    my $meth = $self->lookup_where_dispatch($op) || confess "Invalid operator '$op'";
-   
-    $meth->($self, $ast);
-  }
-
   method _binop(HashAST $ast) {
     my ($lhs, $rhs) = @{$ast->{args}};
     my $op = $ast->{op};
 
-    join (' ', $self->_where_component($lhs), 
+    join (' ', $self->_expr($lhs), 
                $self->binop_mapping($op) || croak("Unknown binary operator $op"),
-               $self->_where_component($rhs)
+               $self->_expr($rhs)
     );
   }
 
-  method _in(ArrayAST $ast) {
-    my ($tag, $field, @values) = @$ast;
+  method _in(HashAST $ast) {
+  
+    my ($field,@values) = @{$ast->{args}};
+
+    my $not = ($ast->{op} =~ /^not_/) ? " NOT" : "";
 
-    my $not = $tag =~ /^-not/ ? " NOT" : "";
+    return $self->_false unless @values;
 
-    return $self->_false if @values == 0;
-    return $self->_where_component($field) .
-           $not. 
+    return $self->_expr($field) .
+           $not . 
            " IN (" .
            join(", ", map { $self->dispatch($_) } @values ) .
            ")";