-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
Add Cuid2 Support #54375
base: 11.x
Are you sure you want to change the base?
Add Cuid2 Support #54375
Conversation
Thanks for submitting a PR! Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface. Pull requests that are abandoned in draft may be closed due to inactivity. |
It looks like a good separate integration package. Here is an example: https://github.com/cybercog/laravel-optimus |
@@ -57,6 +58,7 @@ | |||
"symfony/uid": "^7.0.3", | |||
"symfony/var-dumper": "^7.0.3", | |||
"tijsverkoyen/css-to-inline-styles": "^2.2.5", | |||
"visus/cuid2": "^5.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This maintainer isn't very active. I'd be very hesitant to base the framework on this package's support.
CUID2 values are a real bad primary key for MySQL - which is the most used database with Laravel. And using it as primary key would be the obvious use case. The problem is that MySQL is really slow when the primary key is in random order - you can find tones of information about it. I am therefore against this change, as it would result in many slow Laravel applications thinking to adopt this who don't know of the implications. I don't see any reason why this can't be released as a package and then used by developers who really know what they are doing. |
I feel like this argument is always used if you see anything other than int. I'm not for or against adding custom id support but majority of apps won't even reach 1mil entries which it doesn't matter at that point random or not, and if you use UUIDv7 or something else than v4 MySQL is pain in the ass to use it with anyway. Also Laravel uses string UUIDv4 for notifications by default, wouldn't that also not imply bad performance for a table that most of the time would have more entries than the whole database combined for most apps? |
Yes. But I didn't see this change before it went live. Otherwise I would have said the same. But its not too late to change. Another problem with cuid2 is that the there are constantly new identifiers invented - we can't add them all. And until today, cuid2 has no real widespread usage in the PHP/Laravel world compared to e.g. UUIDs. |
On that I do agree. In any case, random id's all have a place when it comes to security, and I've played around with Cuid2 on MySQL yesterday where even with 46mil row table the insert and read is still within acceptable range, where insert is <20ms and read is as fast as anything else not int related and that amount of entries is to dream for for 99% of apps. |
This adds Cuid2 support and database tests, non breaking changes for existing supported id generation methods, also adds a config to modify the length of the generated Cuid2. This adds helper methods for the Blueprint schema builder including updating
foreignIdFor
anddefaultMorphKeyType
to support Cuid2, and this adds anStr::isCuid
method for validating a valid Cuid2.Cuid2 offers secure, collision-resistant ids optimized for horizontal scaling and performance.
Cuid2 is:
Official Cuid2 Project
The visus/cuid2 package is used for CUID generation, and it requires the gmp php extension.