Fix tests under postgres - we should always put conditions in the WHERE part not in ON constraints because postgres requires that the table has been joined before the condition references it.

4 files changed, 23 insertions, 19 deletions

diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb
index b9bbeed4d5..ab102b2b8f 100644
--- a/activerecord/lib/active_record/associations/association_scope.rb
+++ b/activerecord/lib/active_record/associations/association_scope.rb
@@ -86,11 +86,14 @@ module ActiveRecord
               scope = scope.where(interpolate(condition))
             end
           else
-            scope = scope.joins(join(
-              foreign_table,
-              table[key].eq(foreign_table[foreign_key]),
-              *conditions[i]
-            ))
+            constraint = table[key].eq(foreign_table[foreign_key])
+            join       = join(foreign_table, constraint)
+
+            scope = scope.joins(join)
+
+            unless conditions[i].empty?
+              scope = scope.where(sanitize(conditions[i], table))
+            end
           end
         end
 
diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb
index 7dc6beeede..4121a5b378 100644
--- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb
+++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb
@@ -74,7 +74,7 @@ module ActiveRecord
               foreign_key = reflection.foreign_key
             when :has_and_belongs_to_many
               # Join the join table first...
-              relation = relation.from(join(
+              relation.from(join(
                 table,
                 table[reflection.foreign_key].
                   eq(foreign_table[reflection.active_record_primary_key])
@@ -89,17 +89,20 @@ module ActiveRecord
               foreign_key = reflection.active_record_primary_key
             end
 
-            conditions = self.conditions[i]
+            constraint = table[key].eq(foreign_table[foreign_key])
 
             if reflection.klass.finder_needs_type_condition?
-              conditions += [reflection.klass.send(:type_condition, table)]
+              constraint = table.create_and([
+                constraint,
+                reflection.klass.send(:type_condition, table)
+              ])
             end
 
-            relation = relation.from(join(
-              table,
-              table[key].eq(foreign_table[foreign_key]),
-              *conditions
-            ))
+            relation.from(join(table, constraint))
+
+            unless conditions[i].empty?
+              relation.where(sanitize(conditions[i], table))
+            end
 
             # The current table in this iteration becomes the foreign table in the next
             foreign_table = table
diff --git a/activerecord/lib/active_record/associations/join_helper.rb b/activerecord/lib/active_record/associations/join_helper.rb
index 6474f39503..eae546e76e 100644
--- a/activerecord/lib/active_record/associations/join_helper.rb
+++ b/activerecord/lib/active_record/associations/join_helper.rb
@@ -38,13 +38,11 @@ module ActiveRecord
         name
       end
 
-      def join(table, *conditions)
-        table.create_join(table, table.create_on(sanitize(conditions)), join_type)
+      def join(table, constraint)
+        table.create_join(table, table.create_on(constraint), join_type)
       end
 
-      def sanitize(conditions)
-        table = conditions.first.left.relation
-
+      def sanitize(conditions, table)
         conditions = conditions.map do |condition|
           condition = active_record.send(:sanitize_sql, interpolate(condition), table.table_alias || table.name)
           condition = Arel.sql(condition) unless condition.is_a?(Arel::Node)
diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb
index 0dd407f342..dd450a2a8e 100644
--- a/activerecord/test/cases/associations/nested_through_associations_test.rb
+++ b/activerecord/test/cases/associations/nested_through_associations_test.rb
@@ -272,7 +272,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase
 
   def test_has_many_through_has_many_with_has_many_through_habtm_source_reflection_preload_via_joins
     assert_includes_and_joins_equal(
-      Author.where('comments.id' => comments(:does_it_hurt).id).order('comments.id'),
+      Author.where('comments.id' => comments(:does_it_hurt).id).order('authors.id'),
       [authors(:david), authors(:mary)], :category_post_comments
     )
   end