adding a join source node

Manifest.txt

@@ -38,6 +38,7 @@ lib/arel/nodes/in.rb
 lib/arel/nodes/inner_join.rb
 lib/arel/nodes/insert_statement.rb
 lib/arel/nodes/join.rb
+lib/arel/nodes/join_source.rb
 lib/arel/nodes/less_than.rb
 lib/arel/nodes/less_than_or_equal.rb
 lib/arel/nodes/lock.rb
@@ -103,6 +104,7 @@ test/test_activerecord_compat.rb
 test/test_attributes.rb
 test/test_crud.rb
 test/test_delete_manager.rb
+test/test_factory_methods.rb
 test/test_insert_manager.rb
 test/test_select_manager.rb
 test/test_table.rb

lib/arel/factory_methods.rb

@@ -2,12 +2,12 @@ module Arel
   ###
   # Methods for creating various nodes
   module FactoryMethods
-    def create_join from, to, on = nil, klass = Nodes::InnerJoin
+    def create_join to, constraint = nil, klass = Nodes::InnerJoin
-      klass.new(from, to, on)
+      klass.new(to, constraint)
     end
 
     def create_string_join from, to
-      create_join from, to, nil, Nodes::StringJoin
+      create_join from, to, Nodes::StringJoin
     end
 
     def create_and clauses

lib/arel/nodes.rb

@@ -15,6 +15,7 @@ require 'arel/nodes/less_than'
 require 'arel/nodes/less_than_or_equal'
 require 'arel/nodes/matches'
 require 'arel/nodes/does_not_match'
+require 'arel/nodes/join_source'
 
 require 'arel/nodes/in'
 require 'arel/nodes/not_in'

lib/arel/nodes/join.rb

@@ -1,12 +1,12 @@
 module Arel
   module Nodes
-    class Join < Arel::Nodes::Node
+    class Join < Arel::Nodes::Binary
-      attr_accessor :left, :right, :constraint
 
-      def initialize left, right, constraint
+      alias :single_source :left
-        @left       = left
+      alias :constraint :right
-        @right      = right
+
-        @constraint = constraint
+      def initialize single_source, constraint
+        super
       end
     end
   end

lib/arel/nodes/join_source.rb

@@ -0,0 +1,20 @@
+module Arel
+  module Nodes
+    ###
+    # Class that represents a join source
+    #
+    #   http://www.sqlite.org/syntaxdiagrams.html#join-source
+
+    class JoinSource < Arel::Nodes::Binary
+      def initialize single_source, joinop = []
+        super
+      end
+
+      def initialize_copy other
+        super
+        @left  = @left.clone if @left
+        @right = @right.clone if @right
+      end
+    end
+  end
+end

lib/arel/nodes/select_core.rb

@@ -1,23 +1,31 @@
 module Arel
   module Nodes
     class SelectCore < Arel::Nodes::Node
-      attr_accessor :from, :projections, :wheres, :groups
+      attr_accessor :projections, :wheres, :groups
-      attr_accessor :having
+      attr_accessor :having, :source
-
-      alias :froms= :from=
-      alias :froms :from
 
       def initialize
-        @from        = nil
+        @source      = JoinSource.new nil
         @projections = []
         @wheres      = []
         @groups      = []
         @having      = nil
       end
 
+      def from
+        @source.left
+      end
+
+      def from= value
+        @source.left = value
+      end
+
+      alias :froms= :from=
+      alias :froms :from
+
       def initialize_copy other
         super
-        @from        = @from.clone if @from
+        @source      = @source.clone if @source
         @projections = @projections.clone
         @wheres      = @wheres.clone
         @group       = @groups.clone

lib/arel/nodes/string_join.rb

@@ -1,11 +1,6 @@
 module Arel
   module Nodes
     class StringJoin < Arel::Nodes::Join
-      undef :constraint
-
-      def initialize left, right, on = nil
-        super
-      end
     end
   end
 end

lib/arel/select_manager.rb

@@ -46,7 +46,7 @@ module Arel
     end
 
     def on *exprs
-      @ctx.froms.constraint = Nodes::On.new(collapse(exprs))
+      @ctx.source.right.last.right = Nodes::On.new(collapse(exprs))
       self
     end
 
@@ -66,16 +66,14 @@ module Arel
       # FIXME: this is a hack to support
       # test_with_two_tables_in_from_without_getting_double_quoted
       # from the AR tests.
-      if @ctx.from
-        source = @ctx.from
 
-        if Nodes::SqlLiteral === table && Nodes::Join === source
+      case table
-          source.left = table
+      when Nodes::SqlLiteral, Arel::Table
-          table = source
+        @ctx.source.left = table
-        end
+      when Nodes::Join
+        @ctx.source.right << table
       end
 
-      @ctx.from = table
       self
     end
 
@@ -92,7 +90,8 @@ module Arel
         klass = Nodes::StringJoin
       end
 
-      from create_join(@ctx.from, relation, nil, klass)
+      @ctx.source.right << create_join(relation, nil, klass)
+      self
     end
 
     def having expr
@@ -140,7 +139,7 @@ module Arel
     end
 
     def join_sql
-      return nil unless @ctx.froms
+      return nil if @ctx.source.right.empty?
 
       sql = @visitor.dup.extend(Visitors::JoinSql).accept @ctx
       Nodes::SqlLiteral.new sql

lib/arel/table.rb

@@ -66,7 +66,7 @@ primary_key (#{caller.first}) is deprecated and will be removed in ARel 3.0.0
         klass = Nodes::StringJoin
       end
 
-      from create_join(self, relation, nil, klass)
+      from(self).join(relation, klass)
     end
 
     def group *columns

lib/arel/visitors/depth_first.rb

@@ -39,14 +39,6 @@ module Arel
         visit o.distinct
       end
 
-      def join o
-        visit o.left
-        visit o.right
-        visit o.constraint
-      end
-      alias :visit_Arel_Nodes_InnerJoin :join
-      alias :visit_Arel_Nodes_OuterJoin :join
-
       def nary o
         o.children.each { |child| visit child }
       end
@@ -65,6 +57,7 @@ module Arel
       alias :visit_Arel_Nodes_GreaterThan        :binary
       alias :visit_Arel_Nodes_GreaterThanOrEqual :binary
       alias :visit_Arel_Nodes_In                 :binary
+      alias :visit_Arel_Nodes_InnerJoin          :binary
       alias :visit_Arel_Nodes_LessThan           :binary
       alias :visit_Arel_Nodes_LessThanOrEqual    :binary
       alias :visit_Arel_Nodes_Matches            :binary
@@ -72,6 +65,7 @@ module Arel
       alias :visit_Arel_Nodes_NotIn              :binary
       alias :visit_Arel_Nodes_Or                 :binary
       alias :visit_Arel_Nodes_Ordering           :binary
+      alias :visit_Arel_Nodes_OuterJoin          :binary
       alias :visit_Arel_Nodes_StringJoin         :binary
       alias :visit_Arel_Nodes_TableAlias         :binary
       alias :visit_Arel_Nodes_Values             :binary

lib/arel/visitors/dot.rb

@@ -36,7 +36,6 @@ module Arel
       def visit_Arel_Nodes_TableAlias o
         visit_edge o, "name"
         visit_edge o, "relation"
-        visit_edge o, "columns"
       end
 
       def visit_Arel_Nodes_Sum o
@@ -90,7 +89,7 @@ module Arel
       end
 
       def visit_Arel_Nodes_SelectCore o
-        visit_edge o, "froms"
+        visit_edge o, "source"
         visit_edge o, "projections"
         visit_edge o, "wheres"
       end
@@ -146,6 +145,7 @@ module Arel
       alias :visit_Arel_Nodes_NotIn              :visit_Arel_Nodes_Equality
       alias :visit_Arel_Nodes_DoesNotMatch       :visit_Arel_Nodes_Equality
       alias :visit_Arel_Nodes_Matches            :visit_Arel_Nodes_Equality
+      alias :visit_Arel_Nodes_JoinSource         :visit_Arel_Nodes_Equality
 
       def visit_String o
         @node_stack.last.fields << o

lib/arel/visitors/join_sql.rb

@@ -12,8 +12,7 @@ module Arel
       private
 
       def visit_Arel_Nodes_SelectCore o
-        return '' unless Nodes::Join === o.from
+        o.source.right.map { |j| visit j }.join ' '
-        visit o.from
       end
 
       def visit_Arel_Nodes_StringJoin o
@@ -22,20 +21,6 @@ module Arel
           visit(o.right)
         ].compact.join ' '
       end
-
-      def visit_Arel_Nodes_OuterJoin o
-        [
-          (visit o.left if Nodes::Join === o.left),
-          "LEFT OUTER JOIN #{visit o.right} #{visit o.constraint if o.constraint}"
-        ].compact.join ' '
-      end
-
-      def visit_Arel_Nodes_InnerJoin o
-        [
-          (visit o.left if Nodes::Join === o.left),
-          "INNER JOIN #{visit o.right} #{visit o.constraint if o.constraint}"
-        ].compact.join ' '
-      end
     end
   end
 end

lib/arel/visitors/to_sql.rb

@@ -127,7 +127,7 @@ module Arel
       def visit_Arel_Nodes_SelectCore o
         [
           "SELECT #{o.projections.map { |x| visit x }.join ', '}",
-          ("FROM #{visit o.from}" if o.from),
+          visit(o.source),
           ("WHERE #{o.wheres.map { |x| visit x }.join ' AND ' }" unless o.wheres.empty?),
           ("GROUP BY #{o.groups.map { |x| visit x }.join ', ' }" unless o.groups.empty?),
           (visit(o.having) if o.having),
@@ -217,16 +217,26 @@ module Arel
         "#{visit o.left} NOT LIKE #{visit o.right}"
       end
 
+      def visit_Arel_Nodes_JoinSource o
+        return unless o.left || !o.right.empty?
+
+        [
+          "FROM",
+          (visit(o.left) if o.left),
+          o.right.map { |j| visit j }.join(' ')
+        ].compact.join ' '
+      end
+
       def visit_Arel_Nodes_StringJoin o
         "#{visit o.left} #{visit o.right}"
       end
 
       def visit_Arel_Nodes_OuterJoin o
-        "#{visit o.left} LEFT OUTER JOIN #{visit o.right} #{visit o.constraint}"
+        "LEFT OUTER JOIN #{visit o.left} #{visit o.right}"
       end
 
       def visit_Arel_Nodes_InnerJoin o
-        "#{visit o.left} INNER JOIN #{visit o.right} #{visit o.constraint if o.constraint}"
+        "INNER JOIN #{visit o.left} #{visit o.right if o.right}"
       end
 
       def visit_Arel_Nodes_On o

test/test_factory_methods.rb

@@ -12,9 +12,9 @@ module Arel
       end
 
       def test_create_join
-        join = @factory.create_join :one, :two, :three
+        join = @factory.create_join :one, :two
         assert_kind_of Nodes::Join, join
-        assert_equal :three, join.constraint
+        assert_equal :two, join.constraint
       end
 
       def test_create_on

test/test_select_manager.rb

@@ -292,20 +292,18 @@ module Arel
 
     it 'should create join nodes' do
       relation = Arel::SelectManager.new Table.engine
-      join = relation.create_join 'foo', 'bar', 'baz'
+      join = relation.create_join 'foo', 'bar'
       assert_kind_of Arel::Nodes::InnerJoin, join
       assert_equal 'foo', join.left
       assert_equal 'bar', join.right
-      assert_equal 'baz', join.constraint
     end
 
     it 'should create join nodes with a klass' do
       relation = Arel::SelectManager.new Table.engine
-      join = relation.create_join 'foo', 'bar', 'baz', Arel::Nodes::OuterJoin
+      join = relation.create_join 'foo', 'bar', Arel::Nodes::OuterJoin
       assert_kind_of Arel::Nodes::OuterJoin, join
       assert_equal 'foo', join.left
       assert_equal 'bar', join.right
-      assert_equal 'baz', join.constraint
     end
 
     describe 'join' do
@@ -350,7 +348,7 @@ module Arel
         table   = Table.new :users
         aliaz   = table.alias
         manager = Arel::SelectManager.new Table.engine
-        manager.from Nodes::InnerJoin.new(table, aliaz, table[:id].eq(aliaz[:id]))
+        manager.from Nodes::InnerJoin.new(aliaz, table[:id].eq(aliaz[:id]))
         manager.join_sql.must_be_like %{
           INNER JOIN "users" "users_2" "users"."id" = "users_2"."id"
         }
@@ -360,7 +358,7 @@ module Arel
         table   = Table.new :users
         aliaz   = table.alias
         manager = Arel::SelectManager.new Table.engine
-        manager.from Nodes::OuterJoin.new(table, aliaz, table[:id].eq(aliaz[:id]))
+        manager.from Nodes::OuterJoin.new(aliaz, table[:id].eq(aliaz[:id]))
         manager.join_sql.must_be_like %{
           LEFT OUTER JOIN "users" "users_2" "users"."id" = "users_2"."id"
         }

test/test_table.rb

@@ -14,19 +14,17 @@ module Arel
     end
 
     it 'should create join nodes' do
-      join = @relation.create_join 'foo', 'bar', 'baz'
+      join = @relation.create_join 'foo', 'bar'
       assert_kind_of Arel::Nodes::InnerJoin, join
       assert_equal 'foo', join.left
       assert_equal 'bar', join.right
-      assert_equal 'baz', join.constraint
     end
 
     it 'should create join nodes with a klass' do
-      join = @relation.create_join 'foo', 'bar', 'baz', Arel::Nodes::OuterJoin
+      join = @relation.create_join 'foo', 'bar', Arel::Nodes::OuterJoin
       assert_kind_of Arel::Nodes::OuterJoin, join
       assert_equal 'foo', join.left
       assert_equal 'bar', join.right
-      assert_equal 'baz', join.constraint
     end
 
     it 'should return an insert manager' do

test/visitors/test_depth_first.rb

@@ -66,15 +66,15 @@ module Arel
       end
 
       def test_inner_join
-        join = Nodes::InnerJoin.new :a, :b, :c
+        join = Nodes::InnerJoin.new :a, :b
         @visitor.accept join
-        assert_equal [:a, :b, :c, join], @collector.calls
+        assert_equal [:a, :b, join], @collector.calls
       end
 
       def test_outer_join
-        join = Nodes::OuterJoin.new :a, :b, :c
+        join = Nodes::OuterJoin.new :a, :b
         @visitor.accept join
-        assert_equal [:a, :b, :c, join], @collector.calls
+        assert_equal [:a, :b, join], @collector.calls
       end
 
       [

test/visitors/test_join_sql.rb

@@ -11,9 +11,9 @@ module Arel
       describe 'inner join' do
         it 'should visit left if left is a join' do
           t    = Table.new :users
-          join = Nodes::InnerJoin.new t, t, Nodes::On.new(t[:id])
+          sm   = t.select_manager
-          j2   = Nodes::InnerJoin.new join, t, Nodes::On.new(t[:id])
+          sm.join(t).on(t[:id]).join(t).on(t[:id])
-          @visitor.accept(j2).must_be_like %{
+          sm.join_sql.must_be_like %{
             INNER JOIN "users" ON "users"."id"
             INNER JOIN "users" ON "users"."id"
           }
@@ -23,9 +23,10 @@ module Arel
       describe 'outer join' do
         it 'should visit left if left is a join' do
           t    = Table.new :users
-          join = Nodes::OuterJoin.new t, t, Nodes::On.new(t[:id])
+          sm   = t.select_manager
-          j2   = Nodes::OuterJoin.new join, t, Nodes::On.new(t[:id])
+          sm.join(t, Nodes::OuterJoin).on(t[:id]).join(
-          @visitor.accept(j2).must_be_like %{
+            t, Nodes::OuterJoin).on(t[:id])
+          sm.join_sql.must_be_like %{
             LEFT OUTER JOIN "users" ON "users"."id"
             LEFT OUTER JOIN "users" ON "users"."id"
           }