Introduce decoupled query_constraints for associations - #51
Open
nvasilevski wants to merge 1 commit into
Open
Conversation
Comment on lines
+532
to
+533
| # If the foreign key is an array, set query constraints options | ||
| if options[:foreign_key].is_a?(Array) && !options[:query_constraints] |
There was a problem hiding this comment.
If the idea is that we're separating the two concepts, do we still need to copy the value over?
Author
There was a problem hiding this comment.
good catch, I think we should be able to drop the copy over in favor of concatenation to get the full list of "columns to query by". Let me see what breaks
nvasilevski
force-pushed
the
introduce-decoupled-query-constraints
branch
from
June 19, 2026 20:40
27fce20 to
af51443
Compare
nvasilevski
force-pushed
the
introduce-decoupled-query-constraints
branch
3 times, most recently
from
July 9, 2026 15:06
dd20f24 to
75a844b
Compare
serioushaircut
added a commit
that referenced
this pull request
Jul 9, 2026
…source Add test coverage for a polymorphic has_many :through whose source belongs_to carries the decoupled query_constraints introduced in PR #51. PR #51's only :through coverage used a single-type source (tags_with_decoupled_qc -> Sharded::Tag); the polymorphic shape that motivates the feature (World Delivery Zone -> region -> Country/Province) was untested. The new test confirms PR #51 already handles the polymorphic case correctly: the middle->target join scopes on the tenant/shard column (blog_id) in addition to the polymorphic id (taggable_id), so a row from another shard cannot match on id alone. The only fix required was in the test scaffolding, not the framework: a polymorphic belongs_to derives its type column from the association name, so taggable_with_decoupled_qc looked for a taggable_with_decoupled_qc_type column while the schema uses the taggable_ prefix. Pinning foreign_type: :taggable_type aligns the type column with the taggable_id foreign key. reflection.rb is left untouched. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Orchestrated-by: ae <noreply@shopify.com> Assisted-By: devx/fca4b6fb-9f85-47ab-9f87-e68a1d779f82
nvasilevski
force-pushed
the
introduce-decoupled-query-constraints
branch
4 times, most recently
from
July 27, 2026 14:41
3fd92c6 to
10cf21b
Compare
nvasilevski
force-pushed
the
introduce-decoupled-query-constraints
branch
from
July 27, 2026 15:00
10cf21b to
42f2cf8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 rails#49671 and rails#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's excluded from
nullification by construction - there's no per-column "should I null this
one?" heuristic to get right, because the column was never one of the things
clearing the association touches.
Mental model
query_constraintsis a list of additional columns to match when querying anassociation's targets (loading and preloading). They are layered on top of the
foreign key - the foreign key always participates, because an association can't be
queried without it.
foreign_keyis given → behaves exactly as today.foreign_keyandquery_constraintsare given →foreign_keyhandleswrites, and querying matches on
foreign_key+ the extra columns. This is thedecoupled, nullification-safe mode.
query_constraintsis given without an explicitforeign_key, theforeign key is derived by convention. This is not yet implemented — for now,
provide
foreign_key:explicitly to get the decoupled behavior.query_constraintsis allowed, not rejected - itis de-duplicated.
Nullify / assign behavior (decoupled mode): clearing an association nulls only
the
foreign_key;query_constraintscolumns are never written or nulled, becausethey scope the owner, not point at the target.
The common case: the foreign key stays conventional
Because
query_constraintsno longer redefines the foreign key, the foreign key stayscompletely traditional. Note:
foreign_keyis not yet derived automatically whenquery_constraintsis given — it must be provided explicitly to get the decoupled(nullification-safe) behavior. In the common case you spell it out:
query_constraintsbecomes a small additive layer on top of a conventional foreignkey, rather than something that replaces it. Auto-deriving
foreign_keywhen onlyquery_constraintsis given is left as a follow-up.Two sides of a constraint (advanced)
A column can have a different name on each side.
query_constraintsaccepts symbols(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_key(an FK can't be derived from arenamed pair) - otherwise an
ArgumentErroris raised.Implementation
query_constraints_foreign_key,normalized_query_constraints_mapping, andjoin_query_constraints_{primary,foreign}_key/join_query_constraints_id_for.Join (
join_scope,AssociationScope) and preload paths resolve keys throughthese, falling back to the existing
join_*behavior when noquery_constraintsare present.
active_record_primary_key(and autosave's write key) ignorequery_constraintswhen an explicit
foreign_keyis present, so writes and.joins()keep using thescalar PK/FK.
ThroughReflection/PolymorphicReflection/RuntimeReflectiondelegate the newmethods in lockstep with their
join_*counterparts.Backward compatibility
Existing composite
foreign_key: [...]associations (e.g. CPK) keep working exactlyas today: the columns act as the foreign key and are written and nulled as a unit.
The decoupled, nullification-safe behavior activates only when
query_constraints:is given alongside an explicit
foreign_key:(or a Hash mapping is used).