Skip to content

Localize patch for AR::Relation#primary_key in Rails 8.0+#243

Merged
kymmt90 merged 1 commit intomasterfrom
localize-primary-key-patch
Feb 25, 2026
Merged

Localize patch for AR::Relation#primary_key in Rails 8.0+#243
kymmt90 merged 1 commit intomasterfrom
localize-primary-key-patch

Conversation

@kymmt90
Copy link
Copy Markdown
Member

@kymmt90 kymmt90 commented Feb 5, 2026

Rework of #238.

Problem

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

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 global 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 global override of primary_key, AR::Calculations#ids/#exists? now uses swapped_id instead of bitemporal_id. This is because those methods calls primary_key on self.

This PR redefines #ids and #exists? to override primary_key locally as #find does.

@kymmt90 kymmt90 self-assigned this Feb 5, 2026
@kymmt90 kymmt90 requested review from krororo and osyo-manga February 6, 2026 02:45
@kymmt90 kymmt90 marked this pull request as ready for review February 6, 2026 02:45
@auto-assign auto-assign bot requested review from kumaie-shr and lighty February 6, 2026 02:45
@kymmt90 kymmt90 force-pushed the localize-primary-key-patch branch 2 times, most recently from 4b6ed29 to e92d946 Compare February 12, 2026 06:41
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 localize-primary-key-patch branch from e92d946 to 2179a7d Compare February 20, 2026 10:40
@kymmt90 kymmt90 requested a review from krororo February 20, 2026 11:04
@kymmt90 kymmt90 force-pushed the localize-primary-key-patch branch from 2179a7d to d1c4328 Compare February 24, 2026 05:26
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.

I've confirmed the changes. It looks good to me.

Copy link
Copy Markdown
Collaborator

@osyo-manga osyo-manga left a comment

Choose a reason for hiding this comment

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

Thanks :)

@kymmt90 kymmt90 merged commit f51d798 into master Feb 25, 2026
20 checks passed
@kymmt90 kymmt90 deleted the localize-primary-key-patch branch February 25, 2026 04:57
@kymmt90 kymmt90 mentioned this pull request Mar 2, 2026
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.

3 participants