From 9653da3c7963834eb9ed93bf24837147fa9b0461 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 14 Dec 2010 09:43:19 -0800 Subject: [PATCH] adding a join source node --- Manifest.txt | 2 ++ lib/arel/factory_methods.rb | 6 +++--- lib/arel/nodes.rb | 1 + lib/arel/nodes/join.rb | 12 ++++++------ lib/arel/nodes/join_source.rb | 20 ++++++++++++++++++++ lib/arel/nodes/select_core.rb | 22 +++++++++++++++------- lib/arel/nodes/string_join.rb | 5 ----- lib/arel/select_manager.rb | 19 +++++++++---------- lib/arel/table.rb | 2 +- lib/arel/visitors/depth_first.rb | 10 ++-------- lib/arel/visitors/dot.rb | 4 ++-- lib/arel/visitors/join_sql.rb | 17 +---------------- lib/arel/visitors/to_sql.rb | 16 +++++++++++++--- test/test_factory_methods.rb | 4 ++-- test/test_select_manager.rb | 10 ++++------ test/test_table.rb | 6 ++---- test/visitors/test_depth_first.rb | 8 ++++---- test/visitors/test_join_sql.rb | 13 +++++++------ 18 files changed, 94 insertions(+), 83 deletions(-) create mode 100644 lib/arel/nodes/join_source.rb diff --git a/Manifest.txt b/Manifest.txt index e60978a947..3735d85481 100644 --- a/Manifest.txt +++ b/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 diff --git a/lib/arel/factory_methods.rb b/lib/arel/factory_methods.rb index c9b6c3b791..85862c24c1 100644 --- a/lib/arel/factory_methods.rb +++ b/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 - klass.new(from, to, on) + def create_join to, constraint = nil, klass = Nodes::InnerJoin + 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 diff --git a/lib/arel/nodes.rb b/lib/arel/nodes.rb index c454152d0d..4cc93d7de4 100644 --- a/lib/arel/nodes.rb +++ b/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' diff --git a/lib/arel/nodes/join.rb b/lib/arel/nodes/join.rb index 07f8c98e85..d3307fe0e0 100644 --- a/lib/arel/nodes/join.rb +++ b/lib/arel/nodes/join.rb @@ -1,12 +1,12 @@ module Arel module Nodes - class Join < Arel::Nodes::Node - attr_accessor :left, :right, :constraint + class Join < Arel::Nodes::Binary - def initialize left, right, constraint - @left = left - @right = right - @constraint = constraint + alias :single_source :left + alias :constraint :right + + def initialize single_source, constraint + super end end end diff --git a/lib/arel/nodes/join_source.rb b/lib/arel/nodes/join_source.rb new file mode 100644 index 0000000000..7b739c19ad --- /dev/null +++ b/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 diff --git a/lib/arel/nodes/select_core.rb b/lib/arel/nodes/select_core.rb index e2293f68a2..bd1930364d 100644 --- a/lib/arel/nodes/select_core.rb +++ b/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 :having - - alias :froms= :from= - alias :froms :from + attr_accessor :projections, :wheres, :groups + attr_accessor :having, :source 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 diff --git a/lib/arel/nodes/string_join.rb b/lib/arel/nodes/string_join.rb index b24e91b6b8..b5420e2dfc 100644 --- a/lib/arel/nodes/string_join.rb +++ b/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 diff --git a/lib/arel/select_manager.rb b/lib/arel/select_manager.rb index 1100c29f20..ec1264c3c2 100644 --- a/lib/arel/select_manager.rb +++ b/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 - source.left = table - table = source - end + case table + when Nodes::SqlLiteral, Arel::Table + @ctx.source.left = table + 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 diff --git a/lib/arel/table.rb b/lib/arel/table.rb index bfc667bf2b..a13fd9fa5b 100644 --- a/lib/arel/table.rb +++ b/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 diff --git a/lib/arel/visitors/depth_first.rb b/lib/arel/visitors/depth_first.rb index 49133653c5..9979c8313e 100644 --- a/lib/arel/visitors/depth_first.rb +++ b/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 diff --git a/lib/arel/visitors/dot.rb b/lib/arel/visitors/dot.rb index 4f80f31421..c525a64a89 100644 --- a/lib/arel/visitors/dot.rb +++ b/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 diff --git a/lib/arel/visitors/join_sql.rb b/lib/arel/visitors/join_sql.rb index 427f2bb726..8c31934cd0 100644 --- a/lib/arel/visitors/join_sql.rb +++ b/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 - visit o.from + o.source.right.map { |j| visit j }.join ' ' 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 diff --git a/lib/arel/visitors/to_sql.rb b/lib/arel/visitors/to_sql.rb index 5767fbae8d..b83bf2f6e7 100644 --- a/lib/arel/visitors/to_sql.rb +++ b/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 diff --git a/test/test_factory_methods.rb b/test/test_factory_methods.rb index 89b10236d1..c40615564f 100644 --- a/test/test_factory_methods.rb +++ b/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 diff --git a/test/test_select_manager.rb b/test/test_select_manager.rb index dec1c2d8c7..761ebdaee3 100644 --- a/test/test_select_manager.rb +++ b/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" } diff --git a/test/test_table.rb b/test/test_table.rb index 42362b563e..54f8b04b79 100644 --- a/test/test_table.rb +++ b/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 diff --git a/test/visitors/test_depth_first.rb b/test/visitors/test_depth_first.rb index 7d7324f627..23b011c2eb 100644 --- a/test/visitors/test_depth_first.rb +++ b/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 [ diff --git a/test/visitors/test_join_sql.rb b/test/visitors/test_join_sql.rb index 181ef6c570..b0eba172e6 100644 --- a/test/visitors/test_join_sql.rb +++ b/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]) - j2 = Nodes::InnerJoin.new join, t, Nodes::On.new(t[:id]) - @visitor.accept(j2).must_be_like %{ + sm = t.select_manager + sm.join(t).on(t[:id]).join(t).on(t[:id]) + 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]) - j2 = Nodes::OuterJoin.new join, t, Nodes::On.new(t[:id]) - @visitor.accept(j2).must_be_like %{ + sm = t.select_manager + sm.join(t, Nodes::OuterJoin).on(t[:id]).join( + 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" }