public
Description: Ruby on Rails
Homepage: http://rubyonrails.org
Clone URL: git://github.com/rails/rails.git
Allowed passing arrays-of-strings to :join everywhere. Merge duplicate join 
strings to avoid table aliasing problems.

Signed-off-by: Michael Koziarski <michael@koziarski.com>
[#1077 state:committed]
Pivotal Labs (author)
Tue Sep 23 13:32:17 -0700 2008
NZKoz (committer)
Wed Sep 24 04:26:06 -0700 2008
commit  487758b3b88a38da3a75900839aea03774904fe1
tree    e24091c573d18f80dde8b8dc8c214d4741515b37
parent  70b8ea4fa6f432919340345ae0d5af6aa8f87ec8
...
1576
1577
1578
1579
1580
1581
1582
1583
1584
1585
1586
1587
1588
 
 
 
 
 
 
 
 
 
1589
 
1590
1591
 
1592
1593
1594
...
1604
1605
1606
 
 
 
 
1607
1608
1609
...
1652
1653
1654
1655
1656
 
 
 
 
 
 
1657
1658
1659
...
1576
1577
1578
 
 
 
 
 
 
 
 
 
 
1579
1580
1581
1582
1583
1584
1585
1586
1587
1588
1589
1590
 
1591
1592
1593
1594
...
1604
1605
1606
1607
1608
1609
1610
1611
1612
1613
...
1656
1657
1658
 
 
1659
1660
1661
1662
1663
1664
1665
1666
1667
0
@@ -1576,19 +1576,19 @@ module ActiveRecord #:nodoc:
0
          (safe_to_array(first) + safe_to_array(second)).uniq
0
         end
0
 
0
-        def merge_joins(first, second)
0
-          if first.is_a?(String) && second.is_a?(String)
0
-            "#{first} #{second}"
0
-          elsif first.is_a?(String) || second.is_a?(String)
0
-            if first.is_a?(String)
0
-              join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, second, nil)
0
-              "#{first} #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join}"
0
-            else
0
-              join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, first, nil)
0
-              "#{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} #{second}"
0
+        def merge_joins(*joins)
0
+          if joins.any?{|j| j.is_a?(String) || array_of_strings?(j) }
0
+            joins = joins.collect do |join|
0
+              join = [join] if join.is_a?(String)
0
+              unless array_of_strings?(join)
0
+                join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, join, nil)
0
+                join = join_dependency.join_associations.collect { |assoc| assoc.association_join }
0
+              end
0
+              join
0
             end
0
+            joins.flatten.uniq
0
           else
0
-            (safe_to_array(first) + safe_to_array(second)).uniq
0
+            joins.collect{|j| safe_to_array(j)}.flatten.uniq
0
           end
0
         end
0
 
0
@@ -1604,6 +1604,10 @@ module ActiveRecord #:nodoc:
0
           end
0
         end
0
 
0
+        def array_of_strings?(o)
0
+          o.is_a?(Array) && o.all?{|obj| obj.is_a?(String)}
0
+        end
0
+
0
         def add_order!(sql, order, scope = :auto)
0
           scope = scope(:find) if :auto == scope
0
           scoped_order = scope[:order] if scope
0
@@ -1652,8 +1656,12 @@ module ActiveRecord #:nodoc:
0
           merged_joins = scope && scope[:joins] && joins ? merge_joins(scope[:joins], joins) : (joins || scope && scope[:joins])
0
           case merged_joins
0
           when Symbol, Hash, Array
0
-            join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, merged_joins, nil)
0
-            sql << " #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} "
0
+            if array_of_strings?(merged_joins)
0
+              sql << merged_joins.join(' ') + " "
0
+            else
0
+              join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, merged_joins, nil)
0
+              sql << " #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} "
0
+            end
0
           when String
0
             sql << " #{merged_joins} "
0
           end
...
935
936
937
 
 
 
 
 
 
 
 
 
 
 
938
939
940
...
935
936
937
938
939
940
941
942
943
944
945
946
947
948
949
950
951
0
@@ -935,6 +935,17 @@ class FinderTest < ActiveRecord::TestCase
0
     assert_equal 1, first.id
0
   end
0
 
0
+  def test_joins_with_string_array
0
+    person_with_reader_and_post = Post.find(
0
+      :all,
0
+      :joins => [
0
+        "INNER JOIN categorizations ON categorizations.post_id = posts.id",
0
+        "INNER JOIN categories ON categories.id = categorizations.category_id AND categories.type = 'SpecialCategory'"
0
+      ]
0
+    )
0
+    assert_equal 1, person_with_reader_and_post.size
0
+  end
0
+
0
   def test_find_by_id_with_conditions_with_or
0
     assert_nothing_raised do
0
       Post.find([1,2,3],
...
138
139
140
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
141
142
143
...
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
0
@@ -138,6 +138,36 @@ class MethodScopingTest < ActiveRecord::TestCase
0
     assert_equal authors(:david).attributes, scoped_authors.first.attributes
0
   end
0
 
0
+  def test_scoped_find_merges_string_array_style_and_string_style_joins
0
+    scoped_authors = Author.with_scope(:find => { :joins => ["INNER JOIN posts ON posts.author_id = authors.id"]}) do
0
+      Author.find(:all, :select => 'DISTINCT authors.*', :joins => 'INNER JOIN comments ON posts.id = comments.post_id', :conditions => 'comments.id = 1')
0
+    end
0
+    assert scoped_authors.include?(authors(:david))
0
+    assert !scoped_authors.include?(authors(:mary))
0
+    assert_equal 1, scoped_authors.size
0
+    assert_equal authors(:david).attributes, scoped_authors.first.attributes
0
+  end
0
+
0
+  def test_scoped_find_merges_string_array_style_and_hash_style_joins
0
+    scoped_authors = Author.with_scope(:find => { :joins => :posts}) do
0
+      Author.find(:all, :select => 'DISTINCT authors.*', :joins => ['INNER JOIN comments ON posts.id = comments.post_id'], :conditions => 'comments.id = 1')
0
+    end
0
+    assert scoped_authors.include?(authors(:david))
0
+    assert !scoped_authors.include?(authors(:mary))
0
+    assert_equal 1, scoped_authors.size
0
+    assert_equal authors(:david).attributes, scoped_authors.first.attributes
0
+  end
0
+
0
+  def test_scoped_find_merges_joins_and_eliminates_duplicate_string_joins
0
+    scoped_authors = Author.with_scope(:find => { :joins => 'INNER JOIN posts ON posts.author_id = authors.id'}) do
0
+      Author.find(:all, :select => 'DISTINCT authors.*', :joins => ["INNER JOIN posts ON posts.author_id = authors.id", "INNER JOIN comments ON posts.id = comments.post_id"], :conditions => 'comments.id = 1')
0
+    end
0
+    assert scoped_authors.include?(authors(:david))
0
+    assert !scoped_authors.include?(authors(:mary))
0
+    assert_equal 1, scoped_authors.size
0
+    assert_equal authors(:david).attributes, scoped_authors.first.attributes
0
+  end
0
+
0
   def test_scoped_count_include
0
     # with the include, will retrieve only developers for the given project
0
     Developer.with_scope(:find => { :include => :projects }) do
...
271
272
273
 
 
 
 
 
 
274
...
271
272
273
274
275
276
277
278
279
280
0
@@ -271,4 +271,10 @@ class NamedScopeTest < ActiveRecord::TestCase
0
       topics.size # use loaded (no query)
0
     end
0
   end
0
+
0
+  def test_chaining_with_duplicate_joins
0
+    join = "INNER JOIN comments ON comments.post_id = posts.id"
0
+    post = Post.find(1)
0
+    assert_equal post.comments.size, Post.scoped(:joins => join).scoped(:joins => join, :conditions => "posts.id = #{post.id}").size
0
+  end
0
 end

Comments