Merge pull request #47452 from Shopify/has-many-through-with-query-co…

…nstraints

Support has_many through associations with composite query_constraints

activerecord/lib/active_record/associations/association_scope.rb

@@ -79,19 +79,23 @@ def transform_value(value)
         end
 
         def next_chain_scope(scope, reflection, next_reflection)
-          primary_key = reflection.join_primary_key
-          foreign_key = reflection.join_foreign_key
+          primary_key = Array(reflection.join_primary_key)
+          foreign_key = Array(reflection.join_foreign_key)
 
           table = reflection.aliased_table
           foreign_table = next_reflection.aliased_table
-          constraint = table[primary_key].eq(foreign_table[foreign_key])
+
+          primary_key_foreign_key_pairs = primary_key.zip(foreign_key)
+          constraints = primary_key_foreign_key_pairs.map do |join_primary_key, foreign_key|
+            table[join_primary_key].eq(foreign_table[foreign_key])
+          end.inject(&:and)
 
           if reflection.type
             value = transform_value(next_reflection.klass.polymorphic_name)
             scope = apply_scope(scope, table, reflection.type, value)
           end
 
-          scope.joins!(join(foreign_table, constraint))
+          scope.joins!(join(foreign_table, constraints))
         end
 
         class ReflectionProxy < SimpleDelegator # :nodoc:

activerecord/test/cases/associations/has_many_through_associations_test.rb

@@ -36,12 +36,14 @@
 require "models/section"
 require "models/seminar"
 require "models/session"
+require "models/sharded"
 
 class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   fixtures :posts, :readers, :people, :comments, :authors, :categories, :taggings, :tags,
            :owners, :pets, :toys, :jobs, :references, :companies, :members, :author_addresses,
            :subscribers, :books, :subscriptions, :developers, :categorizations, :essays,
-           :categories_posts, :clubs, :memberships, :organizations, :author_favorites
+           :categories_posts, :clubs, :memberships, :organizations, :author_favorites,
+           :sharded_blog_posts, :sharded_tags, :sharded_blog_posts_tags
 
   # Dummies to force column loads so query counts are clean.
   def setup
@@ -1569,6 +1571,42 @@ def test_circular_autosave_association_correctly_saves_multiple_records
     assert_equal sections, fall.sections.sort_by(&:id)
   end
 
+  def test_post_has_many_tags_through_association_with_composite_query_constraints
+    blog_post = sharded_blog_posts(:great_post_blog_one)
+    expected_tag_ids = Sharded::BlogPostTag.where(blog_post_id: blog_post.id, blog_id: blog_post.blog_id).pluck(:tag_id)
+    tag_ids = []
+    sql = capture_sql do
+      tag_ids = blog_post.tags.to_a.map(&:id)
+    end.first
+
+    c = Sharded::Blog.connection
+    quoted_tags_blog_id = Regexp.escape(c.quote_table_name("sharded_tags.blog_id"))
+    quoted_posts_tags_blog_id = Regexp.escape(c.quote_table_name("sharded_blog_posts_tags.blog_id"))
+    assert_match(/.* ON.* #{quoted_tags_blog_id} = #{quoted_posts_tags_blog_id} .* WHERE/, sql)
+    assert_match(/.* WHERE #{quoted_posts_tags_blog_id} = .*/, sql)
+
+    assert_not_empty(tag_ids)
+    assert_equal(expected_tag_ids.sort, tag_ids.sort)
+  end
+
+  def test_tags_has_manu_posts_through_association_with_composite_query_constraints
+    tag = sharded_tags(:short_read_blog_one)
+    expected_blog_post_ids = Sharded::BlogPostTag.where(tag_id: tag.id, blog_id: tag.blog_id).pluck(:blog_post_id)
+    blog_post_ids = []
+    sql = capture_sql do
+      blog_post_ids = tag.blog_posts.to_a.map(&:id)
+    end.first
+
+    c = Sharded::Blog.connection
+    quoted_blog_posts_blog_id = Regexp.escape(c.quote_table_name("sharded_blog_posts.blog_id"))
+    quoted_posts_tags_blog_id = Regexp.escape(c.quote_table_name("sharded_blog_posts_tags.blog_id"))
+    assert_match(/.* ON.* #{quoted_blog_posts_blog_id} = #{quoted_posts_tags_blog_id} .* WHERE/, sql)
+    assert_match(/.* WHERE #{quoted_posts_tags_blog_id} = .*/, sql)
+
+    assert_not_empty(blog_post_ids)
+    assert_equal(expected_blog_post_ids.sort, blog_post_ids.sort)
+  end
+
   private
     def make_model(name)
       Class.new(ActiveRecord::Base) { define_singleton_method(:name) { name } }

activerecord/test/fixtures/sharded_blog_posts_tags.yml

@@ -0,0 +1,27 @@
+_fixture:
+  model_class: Sharded::BlogPostTag
+
+short_read_first_post_blog_one:
+  tag_id: <%= ActiveRecord::FixtureSet.identify(:short_read_blog_one) %>
+  blog_post_id: <%= ActiveRecord::FixtureSet.identify(:great_post_blog_one) %>
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_one) %>
+
+technical_content_first_post_blog_one:
+  tag_id: <%= ActiveRecord::FixtureSet.identify(:technical_blog_one) %>
+  blog_post_id: <%= ActiveRecord::FixtureSet.identify(:great_post_blog_one) %>
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_one) %>
+
+short_read_second_post_blog_one:
+  tag_id: <%= ActiveRecord::FixtureSet.identify(:short_read_blog_one) %>
+  blog_post_id: <%= ActiveRecord::FixtureSet.identify(:second_post_blog_one) %>
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_one) %>
+
+beginner_content_first_post_blog_two:
+  tag_id: <%= ActiveRecord::FixtureSet.identify(:beginner_blog_two) %>
+  blog_post_id: <%= ActiveRecord::FixtureSet.identify(:great_post_blog_two) %>
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_two) %>
+
+short_read_first_post_blog_two:
+  tag_id: <%= ActiveRecord::FixtureSet.identify(:short_read_blog_two) %>
+  blog_post_id: <%= ActiveRecord::FixtureSet.identify(:great_post_blog_two) %>
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_two) %>

activerecord/test/fixtures/sharded_tags.yml

@@ -0,0 +1,34 @@
+_fixture:
+  model_class: Sharded::Tag
+
+short_read_blog_one:
+  name: "short read"
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_one) %>
+
+long_read_blog_one:
+  name: "long read"
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_one) %>
+
+technical_blog_one:
+  name: "tech"
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_one) %>
+
+time_management_blog_one:
+  name: "time management"
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_one) %>
+
+beginner_blog_two:
+  name: "for beginners"
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_two) %>
+
+intermediate_blog_two:
+  name: "for intermediate"
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_two) %>
+
+short_read_blog_two:
+  name: "short read"
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_two) %>
+
+breaking_news_blog_2:
+  name: "breaking news"
+  blog_id: <%= ActiveRecord::FixtureSet.identify(:sharded_blog_two) %>

activerecord/test/models/sharded.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+require_relative "sharded/blog"
+require_relative "sharded/blog_post"
+require_relative "sharded/comment"
+require_relative "sharded/tag"
+require_relative "sharded/blog_post_tag"

activerecord/test/models/sharded/blog_post.rb

@@ -7,5 +7,8 @@ class BlogPost < ActiveRecord::Base
 
     belongs_to :blog
     has_many :comments, query_constraints: [:blog_id, :blog_post_id]
+
+    has_many :blog_post_tags, query_constraints: [:blog_id, :blog_post_id]
+    has_many :tags, through: :blog_post_tags
   end
 end

activerecord/test/models/sharded/blog_post_tag.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+module Sharded
+  class BlogPostTag < ActiveRecord::Base
+    self.table_name = :sharded_blog_posts_tags
+
+    belongs_to :blog_post, query_constraints: [:blog_id, :blog_post_id]
+    belongs_to :tag, query_constraints: [:blog_id, :tag_id]
+  end
+end

activerecord/test/models/sharded/tag.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module Sharded
+  class Tag < ActiveRecord::Base
+    self.table_name = :sharded_tags
+    query_constraints :blog_id, :id
+
+    has_many :blog_post_tags, query_constraints: [:blog_id, :tag_id]
+    has_many :blog_posts, through: :blog_post_tags
+  end
+end

activerecord/test/schema/schema.rb

@@ -258,6 +258,17 @@
     t.integer :blog_id
   end
 
+  create_table :sharded_tags, force: true do |t|
+    t.string :name
+    t.integer :blog_id
+  end
+
+  create_table :sharded_blog_posts_tags, force: true do |t|
+    t.integer :blog_id
+    t.integer :blog_post_id
+    t.integer :tag_id
+  end
+
   create_table :clubs, force: true do |t|
     t.string :name
     t.integer :category_id