composite-primary-keys / composite_primary_keys

Composite Primary Keys support for Active Record
1.03k stars 350 forks source link

Tracking issue: Rails first class cpk support #583

Closed bf4 closed 2 years ago

bf4 commented 2 years ago

https://discuss.rubyonrails.org/t/rfc-finally-support-composite-primary-keys/81368/7?u=bf4

Nikita Vasilevsky nikita writes

Hey folks :wave: I’m genuinely glad this discussion is happening I wanted to share that at Shopify we are working on proposing features that will help Rails natively support composite primary keys. Can’t speak on behalf the Rails core team, but we will do our best to make those proposals as solid as possible to Rails core team agrees to merge them.

I’ll shamelessly plug our reasoning and performance gains from using composite primary keys on the db-level - How to Introduce Composite Primary Keys in Rails But @ryantownsend provided great insight, essentially any multi-tenant application can potentially benefit from having a tenant_id, id composite primary key. Enforcing most of database queries to a tenant-related table makes it easier to prevent cross-tenant records access. Unless you are performing some kind of a maintenance work, you would most likely want to ensure that the records loaded belong to the “current tenant” within a tenant-scoped request.

We don’t use the composite_primary_keys gem as our use-cases are fairly simple - we maintain the uniqueness of the id column which allows us to select/update/delete records by id only, we don’t have complex associations and the ones we have just work because of the id uniqueness and we also perform most of our queries per single tenant so row-constructor is not needed. But we would still prefer updating our updates like order.touch to include composite primary key clause to avoid gap locks as with InnoDB locking, if a record is being updated at the end of an index (i.e. the largest value) and is not using the PRIMARY KEY or a UNIQUE index, it will block any attempt to insert a record after it for the duration of the transaction containing the UPDATE .

Most of the features we are about to work on won’t even explicitly be related to the composite primary keys but they will provide most of the foundations for the feature. For example row constructor was already proposed to be supported but the only concern was the proposed API. Generally it doesn’t seem that the core team was against the feature. As was mentioned in the PR, presumably an API like Model.where([:tenant_id, :id] => [[1,1], [1,2], [2,3]]) would be a better option. Or, going further and being composite primary key specific, perhaps a helper like Model.primary_key_scope([[1,1], [1,2], [2,3]])) would be a decent higher-level addition to the feature

Also some parts of Rails are almost ready to support composite keys. For example _primary_key_constraints_hash provides solid foundation for making sure that single-object methods like save update update_columns delete destroy reload will use all parts of the composite primary keys in the sql clause

https://github.com/rails/rails/blob/86a280a34781d708180bdcfd9c255192d96dbb77/activerecord/lib/active_record/persistence.rb#L1058

Just wanted to point this out but please don’t monkeypatch it as it’s a private method and Rails can’t guarantee it will always exist :upside_down_face:

To summarize, we are looking into adding composite primary keys support to Rails and hoping that the feature will get accepted. So we welcome all kinds of feedback/questions/collaborations. Thanks! :heart:

and

Eileen M. Uchitelle eileencodes core team

Basically, in order to have a primary key (or any unique constraint really) on a partitioned table, the partition key (eg tenant_id) must be a part of that key. Seems to me like adding composite keys in ActiveRecord would also cover this use case.

This is one of the reasons we’re prioritizing this project at Shopify. The idea was that we can add composite keys for “real” composite key use cases but that it will also be useful for “virtual” composite keys when we need to include a tenant_id in queries for sharding. My hope is that this will improve Rails integration for Vitess and any system that uses partition key based sharding.

cfis commented 2 years ago

Thanks for the tip - I left a comment in that discussion. Should this ticket be left open or closed?

bf4 commented 2 years ago

Your call as a maintainer. I thought it might be useful to cross link the repository with the issue.

cfis commented 2 years ago

Ok cool.