Skip to content

Composite primary keys for some tables #21602

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

Closed
nekromoff opened this issue Oct 10, 2017 · 16 comments
Closed

Composite primary keys for some tables #21602

nekromoff opened this issue Oct 10, 2017 · 16 comments

Comments

@nekromoff
Copy link

It is often just lazy to keep autoincerement id around, specially if thhere are just two other (foreign) id key columns.

| id | user_id | organization_id |

There is really no need for id key column in this case. However, updateOrCreate will fail on tables that do not have id column defined.

Eloquent should be able to work with tables without primary id key including updateOrCreate.

Yes, I know about direct access via DB and the old issue raised in similar request:
#5355

But, two years have passed since that issues has been raised and Laravel and Eloquent have developed.

@Dylan-DPC-zz
Copy link

This repo is for bug tracking. This isn't a bug so you can open a proposal in the laravel/internals repo

@themsaid
Copy link
Member

Please ask on the forums, this repo is for bug reporting only. You can use https://laracasts.com/discuss or https://laravel.io/forum which are forums with a very large community of developers helping each other.

@nekromoff
Copy link
Author

nekromoff commented Oct 10, 2017 via email

@Dylan-DPC-zz
Copy link

As far as I can remember lol 😆

@nekromoff
Copy link
Author

nekromoff commented Oct 10, 2017 via email

@Dylan-DPC-zz
Copy link

If you have maintained a large open source project, you will know the trouble of bugs sitting there in issue board for long time due to loads of "help questions" being posted before that. Also we have the forums and slack channel so that helps us to divide responsibilities and answer questions quickly and get more people involved.

Talking about other repos, you should probably check other repos as well. Laravel isn't the only one doing this.

@nekromoff
Copy link
Author

nekromoff commented Oct 10, 2017 via email

@ConnorVG
Copy link
Contributor

@nekromoff please take this to laravel/internals as advised. Starting an argument in a public forum isn't very professional nor is it very friendly.

The docs are our friend.

@Dylan-DPC-zz
Copy link

I did respond in a respectful way. A little bit of humour isn't dangerous 😉

As far as i remember, Laravel github issues have always been for bug fixes. We have lot of members helping on the forums & the slack channel so I don't see the point of crowding this issue board with help requests.

@gareth-ib
Copy link

so... Laravel already supports creating multiple field primary keys. As seen in their docs on https://laravel.com/docs/6.x/migrations

$table->primary(['id', 'parent_id']);

But after that, eloquent is broken in the sense that it doesn't support that key. So this is an actual bug, not just a feature request

@devcircus
Copy link
Contributor

These keys are useful in situations in which Eloquent support is unnecessary, so to me, it seems like a feature request. Also, it must not be a simple addition because this has been talked about for years.

If it's not already there, it makes sense to note Eloquent's lack of support for composite keys in the docs.

@gareth-ib
Copy link

I have many needs for using eloquent models on tables with composite primary keys right now and historically. But because of the lack of support we can't use it. And obviously other people need it, which is why it's been asked about for years.

As far as it being difficult or not, from what I've seen in the code, it's not looking very complicated. I'd work on it myself if I can get confirmation it would be accepted; because in my experience the Laravel team has been very hard headed about outside ideas. That and people saying we shouldn't do it because it doesn't exist is the catch 22 reason it has only been talked about for years and not implemented.

Not supporting a basic concept that all SQL databases support is a weird thing to intentionally strive to omit from the system.

@devcircus
Copy link
Contributor

That all makes perfect sense. My point was that composite keys CAN be used without Eloquent support. And so it seems like a feature, and maybe even a feature that a lot of people want but still not a bug. So, in the meantime, a note in the docs might help and hopefully someone will figure it out soon.

@gareth-ib
Copy link

And thought process like that is exactly what prevents support of standardized features...
We could use text fields and integers without eloquent too... The discussion is about being able to use eloquent for standard SQL database compatibility.
You saying to note that eloquent can only create composite keys but not use composite keys is a distraction from solving the actual problem. Please focus on helping solve problems rather than saying to ignore them and continue having to do workarounds

@devcircus
Copy link
Contributor

Sounds like you need to open a issue on the ideas repo, which is made for this exact situation. That way you can gauge support before spending time on a pull request. Realistically though if someone doesn't move forward with this. A note in the docs in the meantime wouldn't hurt and could save people from wasted time trying to figure out why it doesn't work. Either way, I'm out. I've never had the need. As a community driven product, the devs that most need a feature tend to drive the development of it. Start in the ideas repo. At least that's a first step in the right direction. ✌🏻️

@gareth-ib
Copy link

gareth-ib commented Nov 4, 2019

yeah it's already an idea issue on laravel/ideas#1699
I've commented on there as well.
as well as older issues on the same subject that have been obstructively closed for no reason like laravel/ideas#712
and this one, again closed just because one contributor didn't personally need the feature, even though people were actively finding detailed solutions.
#5517

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

6 participants