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

Is gem still relevant with Rails 7.1? #608

Open
fragkakis opened this issue Oct 3, 2023 · 14 comments
Open

Is gem still relevant with Rails 7.1? #608

fragkakis opened this issue Oct 3, 2023 · 14 comments

Comments

@fragkakis
Copy link

Rails 7.1 (currently available as RC) supports composite keys.

Does it make sense to use this gem in Rails 7.1 or above? Is the Rails support equivalent, or does the gem offer anything extra?

@codeodor
Copy link
Collaborator

codeodor commented Oct 3, 2023

I probably would not consider using CPK for a new project, if you're just now starting one.

While I haven't checked out Rails 7.1 yet, for my project, if it's easy enough to migrate over, and isn't hampered by performance issues, we'll be doing that. I only plan on keeping CPK updated in our project if it's not easy enough to switch to Rails' version. (Note I am not the decision maker on this -- just giving my 2 cents, having contributed some updates to this gem)

@tomhughes
Copy link
Contributor

As far as I can tell that comment in the release notes is at best misleading - if you read the linked blog post about what Shopify actually did you'll see that while they had a composite primary key in the database they still had a unique ID field which they used as the "primary key" in rails.

I've seen no evidence so far that rails 7.1.0 can actually handle multipart primary keys as a way of identifying records or joining tables - if it is possible then there is no documentation that I can find on how to do it.

@tomhughes
Copy link
Contributor

tomhughes commented Oct 6, 2023

I take it all back - the documentation is at https://guides.rubyonrails.org/active_record_composite_primary_keys.html and it does seem to work.

The bizarre thing is that while you can set primary_key as an array, or let rails determine that automatically, you can't set :foreign_key to an array on a relationship and you have to use :query_constraints instead.

@jarl-dk
Copy link

jarl-dk commented Oct 9, 2023

I agree with you, @tomhughes, the need for query_constraints seem very odd. Like CPK is not a first class citizen after all.
I have tried it out as well. The docs you are referring to illustrates an example of a parent-child association, where the parrent has a CPK. But that is not the most common situation IMHO. the more common situation is that we have a parent-child association where the child has a CPK that is a composition of the PK of the parent plus one row in the child model. That case does not work very well in Rails 7.1. I'll see if I can open an issue on the rails project regarding that.

@jarl-dk
Copy link

jarl-dk commented Oct 9, 2023

I would love to see this gem to support Rails 7.1 until the CPK implementation in Rails is mature.

@tomhughes
Copy link
Contributor

I was able to successfully switch https://github.com/openstreetmap/openstreetmap-website to rails 7.1 with the builtin CPK support in openstreetmap/openstreetmap-website#4278 basically just be either dropping the primary key assignment or changing primary_keys to primary_key and then replacing compound foreign keys with query_constraints in openstreetmap/openstreetmap-website@a0fb8ad.

@cfis
Copy link
Contributor

cfis commented Oct 10, 2023

Thanks for the info @tomhughes. So it seems there is no point in doing a 7.1 release for CPK then?

@jarl-dk
Copy link

jarl-dk commented Oct 12, 2023

FYI: I have opened a bug report on rails regarding composite primary keys:
rails/rails#49597

@istrasci
Copy link

Thanks for the info @tomhughes. So it seems there is no point in doing a 7.1 release for CPK then?

For whatever my vote is worth, I'd still like a 7.1 release and to use this gem for the foreseeable future. I have my own gem that maintains compatibility with several Rails versions. I'd rather not have to put logic in my gem's models just to use different syntax depending on the Rails version. (I also don't want to immediately drop support for all Rails < 7.1.)

@gstokkink
Copy link

gstokkink commented Oct 17, 2023

I'd also like a release of this gem for Rails 7.1, if possible. Best to upgrade in small steps I think, and dropping both this gem and upgrading to Rails 7.1 in one go is too much.

@jarl-dk
Copy link

jarl-dk commented Oct 19, 2023

Based on rails/rails#49622 (comment), I also also suggest maintaining this gem until Rails 8.

@Little-Rubyist
Copy link

Hello!
We are using CPK and attempting to upgrade to Rails 7.1.
However, due to the current limitation that ibmdb only supports up to Rails 7.0, it seems challenging to upgrade straightforwardly.

I'm curious to know if there are plans for CPK to support Rails 7.1.

@cfis
Copy link
Contributor

cfis commented Dec 28, 2023

The comments above indicate that switching to Rails built-in support for composite keys is not hard to do. In addition, upgrading CPK for major new Rails versions is quite difficult and time consuming (at least days of work if not a week or two). And I suspect this upgrade might be quite difficult since Rails 7.1 has likely changed most of the code CPK changes.

Between those two facts, I hope it will not be necessary to upgrade this gem for 7.1. So I don't plan on updating CPK for Rails 7.1. I am happy to review PRs if someone wants to try though.

@jarl-dk
Copy link

jarl-dk commented Mar 4, 2024

Yet another reason for maintaining this gem (until Rails has matured on CPK): rails/rails#51231

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

No branches or pull requests

8 participants