Skip to content

Remove patch for AR::Relation#primary_key in Rails 8.0+#238

Closed
kymmt90 wants to merge 2 commits intomasterfrom
remove-primary-key-patch
Closed

Remove patch for AR::Relation#primary_key in Rails 8.0+#238
kymmt90 wants to merge 2 commits intomasterfrom
remove-primary-key-patch

Conversation

@kymmt90
Copy link
Copy Markdown
Member

@kymmt90 kymmt90 commented Jan 15, 2026

Problem

Only AR::Relation's primary_key is overridden with bitemporal_id_key.

def primary_key
bitemporal_id_key
end

Therefore, when model.primary_key is called in Rails 8.0+'s finder methods, it returns the original primary_key value instead of bitemporal_id_key. Additionally, this override can cause various other confusions.

Changes

This PR removes the override of AR::Relation#primary_key in Rails 8.0+. The order of AR objects fetched by finder methods is kept as before thanks to #237.

By removing the override of primary_key, AR::Calculations#ids now returns an array of swapped_id instead of bitemporal_id. This is because AR::Relation::Calculations#ids calls primary_key on self.

https://github.com/rails/rails/blob/v8.0.4/activerecord/lib/active_record/relation/calculations.rb#L371

This PR redefines #ids to keep the previous behavior.

@kymmt90 kymmt90 self-assigned this Jan 15, 2026
@auto-assign auto-assign bot requested review from kumaie-shr and yono January 15, 2026 10:54
@kymmt90 kymmt90 removed request for kumaie-shr and yono January 15, 2026 10:54
@kymmt90 kymmt90 force-pushed the remove-primary-key-patch branch 2 times, most recently from 5fc6a13 to fb14c89 Compare January 15, 2026 11:05
Copy link
Copy Markdown
Collaborator

@krororo krororo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@kymmt90 kymmt90 force-pushed the remove-primary-key-patch branch from fb14c89 to 3b25dcf Compare January 16, 2026 09:11
@kymmt90 kymmt90 force-pushed the remove-primary-key-patch branch from 3b25dcf to ae42c1b Compare January 21, 2026 10:29
@kymmt90
Copy link
Copy Markdown
Member Author

kymmt90 commented Jan 21, 2026

@kymmt90
Copy link
Copy Markdown
Member Author

kymmt90 commented Feb 5, 2026

Reworked on #243.

@kymmt90 kymmt90 closed this Feb 5, 2026
@kymmt90 kymmt90 deleted the remove-primary-key-patch branch February 5, 2026 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants