Introduce decoupled query constraints for associations - #58266
Introduce decoupled query constraints for associations#58266nvasilevski wants to merge 4 commits into
Conversation
4e25193 to
f6b0b1f
Compare
8ffa9b3 to
83a2a24
Compare
Reintroduce query_constraints as a query-only association option separate from foreign_key, including joins, preloading, and legacy composite-key compatibility. Assisted-By: devx/43280c01-a333-4b61-b964-1493f6ac271d
747d9f5 to
79f5ce6
Compare
adrianna-chang-shopify
left a comment
There was a problem hiding this comment.
When query_constraints is given without an explicit foreign_key, the
foreign key is derived by convention. This is not yet implemented
I'm a little confused here. It seems (from me roughly tracing through the code 😅 ) that specifying query_constraints without a foreign gives us the legacy behaviour of turning it into the set of columns to use for both reading and writing.
This is what Pi is telling me:
(In reflection.rb)
foreign_key: [:blog_id, :blog_post_id] (array form):
- Line 535: [:blog_id, :blog_post_id].is_a?(Array) && !nil → true
- Line 536: options[:query_constraints] = options.delete(:foreign_key)
- Result: options = { query_constraints: [:blog_id, :blog_post_id] }, no foreign_key key
query_constraints: [:blog_id, :blog_post_id] (standalone):
- Line 535: nil.is_a?(Array) → false → skip
- Result: options = { query_constraints: [:blog_id, :blog_post_id] }, no foreign_key key
The PR description makes it sound like auto-deriving FK from query_constraints (with no foreign_key: specified) is a future feature, but the code implies otherwise? Shouldn't we continue to guard against the legacy behaviour / using query_constraints without a foreign_key specified?
that shouldn't be the case, at least that's wasn't intended design. To be specific, let's consider a typical association such as What we want to support: Comment.belongs_to :post, foreign_key: :post_id, primary_key: :id, query_constraints: :blog_idwhich ideally should look like: Comment.belongs_to :post, query_constraints: :blog_idwhich can be read as "id/post_id pk/fk pair for association (write) purposes and use blog_id as an additional column to pk/fk pair for querying purposes". id/post_id can be ommited because they are conventional and rails should be able to derive them. what we don't want to allow is: Comment.belongs_to :post, query_constraints: [:blog_id, :post_id]because it makes it look like association doesn't have foreign key which is nonsensical for an association while clearly I pushed a commit that should address that but I'm going to evaluate whole proposal against the expectation described above |
Introduce decoupled
query_constraintsfor associationsReintroduces
query_constraintson associations, decoupled fromforeign_key.Motivation
query_constraintsis deprecated, and the way to scope abelongs_toby anextra column (a shard or tenant) today is to list it inside
foreign_keyas anarray:
Active Record treats every column in that array as part of the foreign key, so
the tenant column (
account_id) gets nulled on clear and reassigned on set(see #49671 and #57906).
This change reintroduces
query_constraintsas a query-only option layeredon top of
foreign_key, separate from it. With the shard column expressed asadditive query scope rather than part of the foreign key, it is excluded from
nullification by construction: the column was never one of the things clearing
the association touches.
Mental model
query_constraintsdeclares additional columns to match when querying anassociation's targets. They participate in loading, preloading, eager loading,
joins, and association predicates. The foreign key always participates because
an association cannot be queried without it.
foreign_keyis given, behavior is unchanged.query_constraintsis given,foreign_keyhandles writes while queriesmatch on the foreign key plus the additional columns.
query_constraintsis given without an explicitforeign_key, theforeign key is derived by convention.
columns; overlapping configurations raise
ArgumentErrorwhen the reflectionis validated.
Nullify / assign behavior: clearing an association nulls only the
foreign_key;query_constraintscolumns are never written or nulled becausethey scope the owner rather than point at the target.
The common case: the foreign key stays conventional
Because
query_constraintsno longer redefines the foreign key, the common caseonly needs to name the additional query column:
This is equivalent to spelling out the conventional key:
An explicit
foreign_keyremains available for non-conventional associations.Two sides of a constraint (advanced)
A column can have a different name on each side.
query_constraintsacceptssymbols (same name on both sides) and hashes (
self_column => target_column):Resulting join keys:
BlogPost) columns:["blog_id", "id", "featured_comment_id"]Comment) columns:["blog_id", "blog_post_id", "id"]A Hash mapping requires an explicit
foreign_keybecause a foreign key cannotbe derived from a renamed pair.
Implementation
query_constraints_foreign_key,normalized_query_constraints_mapping, andjoin_query_constraints_{primary,foreign}_key/join_query_constraints_id_forseparate writable keys from query keys.join_scope,AssociationScope) and preload paths resolve keys throughthe query-constraint methods, falling back to existing
join_*behavior whenno association query constraints are present.
foreign_key,active_record_primary_key, and autosave continue to expose andwrite only the explicit or conventionally derived foreign key.
foreign_keyoptions retain their composite writable-keysemantics; their origin is tracked separately from user-supplied
query_constraints.ThroughReflection,PolymorphicReflection, andRuntimeReflectiondelegatethe new methods in lockstep with their
join_*counterparts.Backward compatibility
Existing composite
foreign_key: [...]associations, including compositeprimary key associations, keep working as today: all columns remain part of the
foreign key and are written and nulled as a unit.
Associations without
query_constraintsare unchanged. Whenquery_constraintsis present, decoupled behavior applies with either anexplicit or conventionally derived foreign key. Hash mappings still require an
explicit
foreign_key.