Introduce decoupled query_constraints for associations

activerecord/CHANGELOG.md

@@ -1,3 +1,25 @@
+*   Support `query_constraints` on associations, decoupled from `foreign_key`.
+
+    `query_constraints` declares *additional* columns to match when querying an
+    association's targets (loading and preloading), layered on top of the foreign
+    key. When both `foreign_key` and `query_constraints` are given, the foreign key
+    handles writes while querying matches on the foreign key plus the extra columns.
+
+    A column may map to a different name on the other side using a Hash, and
+    listing the foreign key itself is allowed (it is de-duplicated):
+
+    ``` ruby
+    class BlogPost < ApplicationRecord
+      # match blog_id on both tables, and BlogPost#id -> Comment#blog_post_id
+      belongs_to :featured_comment,
+        class_name: "Comment",
+        foreign_key: :featured_comment_id,
+        query_constraints: [:blog_id, { id: :blog_post_id }]
+    end
+    ```
+
+    *Nikita Vasilevsky*
+
 *   Report PostgreSQL default timestamp and time precision as 6.
 
     Bare PostgreSQL `timestamp` and `time` columns now use their effective

activerecord/lib/active_record/associations/association_scope.rb

@@ -36,7 +36,7 @@ def self.get_bind_values(owner, chain)
         binds = []
         last_reflection = chain.last
 
-        binds.push(*last_reflection.join_id_for(owner))
+        binds.push(*last_reflection.join_query_constraints_id_for(owner))
         if last_reflection.type
           binds << owner.class.polymorphic_name
         end
@@ -57,8 +57,8 @@ def join(table, constraint)
         end
 
         def last_chain_scope(scope, reflection, owner)
-          primary_key = Array(reflection.join_primary_key)
-          foreign_key = Array(reflection.join_foreign_key)
+          primary_key = Array(reflection.join_query_constraints_primary_key)
+          foreign_key = Array(reflection.join_query_constraints_foreign_key)
 
           table = reflection.aliased_table
           primary_key_foreign_key_pairs = primary_key.zip(foreign_key)
@@ -80,8 +80,8 @@ def transform_value(value)
         end
 
         def next_chain_scope(scope, reflection, next_reflection)
-          primary_key = Array(reflection.join_primary_key)
-          foreign_key = Array(reflection.join_foreign_key)
+          primary_key = Array(reflection.join_query_constraints_primary_key)
+          foreign_key = Array(reflection.join_query_constraints_foreign_key)
 
           table = reflection.aliased_table
           foreign_table = next_reflection.aliased_table

activerecord/lib/active_record/associations/errors.rb

@@ -190,9 +190,9 @@ class CompositePrimaryKeyMismatchError < ActiveRecordError # :nodoc:
     def initialize(reflection = nil)
       if reflection
         if reflection.has_one? || reflection.collection?
-          super("Association #{reflection.active_record}##{reflection.name} primary key #{reflection.active_record_primary_key} doesn't match with foreign key #{reflection.foreign_key}. Please specify query_constraints, or primary_key and foreign_key values.")
+          super("Association #{reflection.active_record}##{reflection.name} primary key `#{reflection.active_record_primary_key}` doesn't match with foreign key `#{reflection.foreign_key}`. Please specify query_constraints, or primary_key and foreign_key values.")
         else
-          super("Association #{reflection.active_record}##{reflection.name} primary key #{reflection.association_primary_key} doesn't match with foreign key #{reflection.foreign_key}. Please specify query_constraints, or primary_key and foreign_key values.")
+          super("Association #{reflection.active_record}##{reflection.name} primary key `#{reflection.association_primary_key}` doesn't match with foreign key `#{reflection.foreign_key}`. Please specify query_constraints, or primary_key and foreign_key values.")
         end
       else
         super("Association primary key doesn't match with foreign key.")

activerecord/lib/active_record/associations/preloader/association.rb

@@ -159,7 +159,7 @@ def preloaded_records
 
         # The name of the key on the associated records
         def association_key_name
-          reflection.join_primary_key(klass)
+          reflection.join_query_constraints_primary_key(klass)
         end
 
         def loader_query
@@ -239,7 +239,7 @@ def associate_records_from_unscoped(unscoped_records)
 
           # The name of the key on the model which declares the association
           def owner_key_name
-            reflection.join_foreign_key
+            reflection.join_query_constraints_foreign_key
           end
 
           def associate_records_to_owner(owner, records)

activerecord/lib/active_record/reflection.rb

@@ -208,8 +208,8 @@ def join_scope(table, foreign_table, foreign_klass)
 
         scope_chain_items.inject(klass_scope, &:merge!)
 
-        primary_key_column_names = Array(join_primary_key)
-        foreign_key_column_names = Array(join_foreign_key)
+        primary_key_column_names = Array(join_query_constraints_primary_key)
+        foreign_key_column_names = Array(join_query_constraints_foreign_key)
 
         primary_foreign_key_pairs = primary_key_column_names.zip(foreign_key_column_names)
 
@@ -528,15 +528,9 @@ def initialize(name, scope, options, active_record)
         @foreign_key = nil
         @association_foreign_key = nil
         @association_primary_key = nil
-        if options[:query_constraints]
-          raise ConfigurationError, <<~MSG.squish
-            Setting `query_constraints:` option on `#{active_record}.#{macro} :#{name}` is not allowed.
-            To get the same behavior, use the `foreign_key` option instead.
-          MSG
-        end
 
-        # If the foreign key is an array, set query constraints options and don't use the foreign key
-        if options[:foreign_key].is_a?(Array)
+        # If the foreign key is an array, set query constraints options
+        if options[:foreign_key].is_a?(Array) && !options[:query_constraints]
           options[:query_constraints] = options.delete(:foreign_key)
         end
 
@@ -559,11 +553,57 @@ def join_table
         @join_table ||= -(options[:join_table]&.to_s || derive_join_table)
       end
 
+      # Normalizes the `query_constraints` option into [self_column, target_column] pairs.
+      # Returns nil for old-style query_constraints (plain FK column arrays without explicit foreign_key).
+      #
+      # Activated when `foreign_key` is also specified, or when any element is a Hash.
+      # Key = column on this model's table (self), Value = column on the other table (target).
+      #
+      # Examples:
+      #   query_constraints: :shop_id                        => [["shop_id", "shop_id"]]
+      #   query_constraints: [:blog_id, { id: :blog_post_id }] => [["blog_id", "blog_id"], ["id", "blog_post_id"]]
+      def normalized_query_constraints_mapping
+        return unless options[:query_constraints]
+        return unless options[:foreign_key] || Array(options[:query_constraints]).any?(Hash)
+
+        @normalized_query_constraints_mapping ||= Array(options[:query_constraints]).flat_map { |element|
+          case element
+          when Symbol, String
+            [[element.to_s, element.to_s]]
+          when Hash
+            element.map { |self_col, target_col| [self_col.to_s, target_col.to_s] }
+          end
+        }.freeze
+      end
+
+      # The set of columns used to query association targets (loading, preloading).
+      #
+      # `query_constraints` is a list of *additional* columns layered on top of the
+      # `foreign_key` — the foreign key always participates, since an association
+      # cannot be queried without it. Listing the foreign key in `query_constraints`
+      # is allowed and harmless: it is de-duplicated here rather than rejected.
+      #
+      # Defaults to `foreign_key` when no `query_constraints` are given.
+      def query_constraints_foreign_key
+        @query_constraints_foreign_key ||= if options[:query_constraints]
+          qc = Array(options[:query_constraints]).map { |fk| fk.to_s.freeze }
+          [*qc, *Array(foreign_key)].uniq.freeze
+        else
+          foreign_key
+        end
+      end
+
       def foreign_key(infer_from_inverse_of: true)
         @foreign_key ||= if options[:foreign_key]
           ActiveRecord::Key.for(options[:foreign_key]).name
         elsif options[:query_constraints]
-          options[:query_constraints].map { |fk| -fk.to_s.freeze }.freeze
+          qc = Array(options[:query_constraints])
+          if qc.any?(Hash)
+            raise ArgumentError,
+              "`query_constraints` with column mapping (Hash) on `#{active_record}.#{macro} :#{name}` " \
+              "requires an explicit `foreign_key` option."
+          end
+          qc.map { |fk| -fk.to_s.freeze }.freeze
         else
           derived_fk = derive_foreign_key(infer_from_inverse_of: infer_from_inverse_of)
 
@@ -583,15 +623,38 @@ def association_primary_key(klass = nil)
         primary_key(klass || self.klass)
       end
 
+      def active_record_query_constraints_primary_key
+        if options[:primary_key]
+          active_record_primary_key
+        elsif active_record.has_query_constraints? || options[:query_constraints]
+          @active_record_query_constraints_primary_key ||= active_record.query_constraints_list
+        else
+          active_record_primary_key
+        end
+      end
+
       def active_record_primary_key
         @active_record_primary_key ||=
           if options[:primary_key]
             ActiveRecord::Key.for(options[:primary_key]).name
+          elsif (active_record.has_query_constraints? || options[:query_constraints]) && !options[:foreign_key]
+            # query_constraints drive the key only when no foreign_key is given;
+            # an explicit foreign_key handles writes and takes precedence here.
+            active_record.query_constraints_list
           else
-            derive_primary_key(active_record) { |model| model.query_constraints_list }
+            active_record.primary_key_definition.inferred_id || primary_key(active_record).freeze
           end
       end
 
+      def join_query_constraints_primary_key(klass = nil)
+        if (mapping = normalized_query_constraints_mapping)
+          # target (child) columns: mapping values + foreign_key
+          [*mapping.map(&:last), *Array(foreign_key)].uniq.freeze
+        else
+          query_constraints_foreign_key
+        end
+      end
+
       def join_primary_key(klass = nil)
         foreign_key
       end
@@ -600,6 +663,15 @@ def join_primary_type
         type
       end
 
+      def join_query_constraints_foreign_key
+        if (mapping = normalized_query_constraints_mapping)
+          # self (parent) columns: mapping keys + active_record_primary_key
+          [*mapping.map(&:first), *Array(active_record_primary_key)].uniq.freeze
+        else
+          active_record_query_constraints_primary_key
+        end
+      end
+
       def join_foreign_key
         active_record_primary_key
       end
@@ -632,6 +704,10 @@ def check_eager_loadable!
         end
       end
 
+      def join_query_constraints_id_for(owner)
+        Array(join_query_constraints_foreign_key).map { |key| owner._read_attribute(key) }
+      end
+
       def join_id_for(owner) # :nodoc:
         Array(join_foreign_key).map { |key| owner._read_attribute(key) }
       end
@@ -815,11 +891,14 @@ def scope_allows_automatic_inverse_of?(reflection, inverse_reflection)
           end
         end
 
-        # Shared by +active_record_primary_key+ and +association_primary_key+ to
-        # resolve the key from +model+ once a custom +primary_key+ is ruled out.
+        # Resolves the key from +model+ once a custom +primary_key+ is ruled out.
         # The block is yielded +model+ to supply its query-constraints list.
+        #
+        # Note: callers handle the association-level +query_constraints+ option
+        # themselves, since its meaning differs by side (and it is decoupled from
+        # +foreign_key+). This only considers the model-level query constraints.
         def derive_primary_key(model)
-          if model.has_query_constraints? || options[:query_constraints]
+          if model.has_query_constraints?
             yield model
           else
             # inferred_id is nil unless the key is composite; otherwise fall back
@@ -933,6 +1012,20 @@ def association_class
         end
       end
 
+      def association_query_constraints_primary_key(klass = nil)
+        # An explicit `primary_key`/`foreign_key` pins a single-column join on the
+        # target's writable key, so defer to +association_primary_key+ even when the
+        # target is query-constrained (e.g. a polymorphic FK to a sharded model).
+        if !options[:primary_key] && !options[:foreign_key] &&
+            ((klass || self.klass).has_query_constraints? || options[:query_constraints])
+          # Not memoized: +klass+ varies for polymorphic associations, so each
+          # target may resolve to a different query-constraints list.
+          (klass || self.klass).query_constraints_list
+        else
+          association_primary_key(klass)
+        end
+      end
+
       # klass option is necessary to support loading polymorphic associations
       def association_primary_key(klass = nil)
         if options[:primary_key]
@@ -948,17 +1041,46 @@ def association_primary_key(klass = nil)
 
         klass ||= self.klass
 
-        if klass.has_query_constraints? && options[:foreign_key] && !options[:query_constraints]
+        # An explicit `foreign_key` handles writes, so the association's writable
+        # key is the target's single primary key even when extra `query_constraints`
+        # are layered on for reads (the decoupled query-constraints feature).
+        if klass.has_query_constraints? && options[:foreign_key]
           return klass.primary_key_definition.inferred_id || primary_key(klass).freeze
         end
 
+        # `query_constraints` without an explicit `foreign_key` (e.g. a composite
+        # array `foreign_key` normalized into `query_constraints`) keys off the
+        # target's declared primary key, which may itself be composite.
+        if options[:query_constraints]
+          return primary_key(klass)
+        end
+
         derive_primary_key(klass) { |model| model.composite_query_constraints_list }
       end
 
+      def join_query_constraints_primary_key(klass = nil)
+        if (mapping = normalized_query_constraints_mapping)
+          # target (parent) columns: mapping values + association_primary_key
+          target_klass = polymorphic? ? klass : (klass || self.klass)
+          [*mapping.map(&:last), *Array(association_primary_key(target_klass))].uniq.freeze
+        else
+          polymorphic? ? association_query_constraints_primary_key(klass) : association_query_constraints_primary_key
+        end
+      end
+
       def join_primary_key(klass = nil)
         polymorphic? ? association_primary_key(klass) : association_primary_key
       end
 
+      def join_query_constraints_foreign_key
+        if (mapping = normalized_query_constraints_mapping)
+          # self (child) columns: mapping keys + foreign_key
+          [*mapping.map(&:first), *Array(foreign_key)].uniq.freeze
+        else
+          query_constraints_foreign_key
+        end
+      end
+
       def join_foreign_key
         foreign_key
       end
@@ -984,8 +1106,13 @@ def collection?
     # Holds all the metadata about a :through association as it was specified
     # in the Active Record class.
     class ThroughReflection < AbstractReflection # :nodoc:
+      # The `join_query_constraints_*` methods must be delegated the same way as
+      # their `join_*` counterparts (foreign key and id-for to +source_reflection+
+      # here, primary key overridden below), otherwise query-constraint joins would
+      # resolve to the wrong reflection on the chain. Keep the two sets in sync.
       delegate :foreign_key, :foreign_type, :association_foreign_key, :join_id_for, :type,
-               :active_record_primary_key, :join_foreign_key, to: :source_reflection
+               :active_record_primary_key, :join_foreign_key,
+               :join_query_constraints_id_for, :join_query_constraints_foreign_key, to: :source_reflection
 
       def initialize(delegate_reflection)
         super()
@@ -1106,6 +1233,10 @@ def association_primary_key(klass = nil)
         end
       end
 
+      def join_query_constraints_primary_key(klass = self.klass)
+        source_reflection.join_query_constraints_primary_key(klass)
+      end
+
       def join_primary_key(klass = self.klass)
         source_reflection.join_primary_key(klass)
       end
@@ -1263,8 +1394,7 @@ def collect_deprecated_nested_reflections
     end
 
     class PolymorphicReflection < AbstractReflection # :nodoc:
-      delegate :klass, :scope, :plural_name, :type, :join_primary_key, :join_foreign_key,
-               :name, :scope_for, to: :@reflection
+      delegate :klass, :scope, :plural_name, :type, :join_primary_key, :join_foreign_key, :name, :scope_for, :join_query_constraints_primary_key, :join_query_constraints_foreign_key, to: :@reflection
 
       def initialize(reflection, previous_reflection)
         super()
@@ -1293,7 +1423,7 @@ def source_type_scope
     end
 
     class RuntimeReflection < AbstractReflection # :nodoc:
-      delegate :scope, :type, :constraints, :join_foreign_key, to: :@reflection
+      delegate :scope, :type, :constraints, :join_foreign_key, :join_query_constraints_foreign_key, to: :@reflection
 
       def initialize(reflection, association)
         super()
@@ -1309,6 +1439,10 @@ def aliased_table
         klass.arel_table
       end
 
+      def join_query_constraints_primary_key(klass = self.klass)
+        @reflection.join_query_constraints_primary_key(klass)
+      end
+
       def join_primary_key(klass = self.klass)
         @reflection.join_primary_key(klass)
       end