Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

activerecord: Extract primary key constraints hash to a method #41427

Merged

Conversation

dylanahsmith
Copy link
Contributor

@dylanahsmith dylanahsmith commented Feb 12, 2021

cc @rafaelfranca

Extract the primary key constraints hash, used for the WHERE scope to update or delete a record, into a method. This is just a small step towards supporting a composite primary key, where we would want constraints on all primary key columns.

Context

We have a use case for using a composite primary key for performance reasons, which isn't actually supported in rails at the moment. Specifically, we use a parent record id along with the actual id to keep the associated records together for efficient querying (e.g. (parent_id, id)).

We can use self.primary_key = :id in the model to make most things work, along with a secondary (id) index. However, that will end up taking gap locks in the innodb mysql engine when there is only an equality constraint on id, causing unnecessary lock contention.

Copy link

@jarthorn jarthorn left a comment

Choose a reason for hiding this comment

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

There is one more reference in update_columns that can use the same hash: https://github.com/rails/rails/blob/main/activerecord/lib/active_record/persistence.rb#L697

Since this hash will need to change in the future to support composite
primary keys.
@dylanahsmith dylanahsmith force-pushed the dry-primary-key-constraints-hash branch from a5faee1 to 1d3ef1a Compare February 12, 2021 20:21
@dylanahsmith
Copy link
Contributor Author

Changed update_columns to also use the extracted method. Thanks

@rafaelfranca rafaelfranca merged commit 1acde22 into rails:main Feb 12, 2021
@dylanahsmith dylanahsmith deleted the dry-primary-key-constraints-hash branch February 17, 2021 20:49
kamipo added a commit that referenced this pull request Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants