search

spatie / laravel-medialibrary

Associate files with Eloquent models
https://spatie.be/docs/laravel-medialibrary
MIT License
5.78k stars 1.08k forks source link

Add Support to `ULID` column type #3699

Closed devajmeireles closed 1 week ago

devajmeireles commented 2 months ago

Based on several studies and personal tests, we can prove the effectiveness of ulid columns instead of uuid when talking about a large volume of data.

A package like this tends to persist a thousand of data in the database since all the media of an entire application is persisted in a table. Some public studies point to abstracts where ULID is advantageous in these cases.

"ULID is often considered superior to UUID when ordering and efficiency are important. It is lexicographically orderable, which makes it easier to organize data in chronological order, something that UUIDs cannot do efficiently. In addition, ULID is more compact, at 26 characters, compared to UUID's 36 characters, making it easier to read and transmit. Embedding a timestamp in ULID means that IDs generated at close points in time will be ordered in a natural way, which can improve database performance by minimizing index fragmentation. While UUIDs are widely used and supported, ULID offers specific advantages in terms of temporal ordering and storage efficiency, making it a preferred choice in many systems that prioritize these characteristics."

A good read is here: https://medium.com/@sammaingi5/uuid-vs-ulid-how-ulid-improves-write-speeds-d16b23505458

This PR is an attempt to implement a configuration that allows ULID to be used instead of UUID.

devajmeireles commented 2 months ago

Hey, @freekmurze ! I'd like to know your opinion on this before I continue with the PR :D There doesn't seem to be any breaking changes, but I'd like your opinion on whether I should move forward because it's something interesting in your eyes.

patinthehat commented 2 months ago

@devajmeireles please feel free to continue with this PR. I'm happy to review it once it's ready :+1: Thank you!

devajmeireles commented 2 months ago

@devajmeireles please feel free to continue with this PR. I'm happy to review it once it's ready 👍 Thank you!

Hey, @patinthehat ! Thanks for your feedback. I think the PR is ready for review. Feel free to review it and point out anything misaligned or incorrect so I can adjust it.

devajmeireles commented 1 month ago

Hey, @patinthehat ! I applied all your suggestions. I also tried searching for UUID (or variations, like: uuid) in the docs, to write something special for someone who wants to use ULID instead of UUID, but the docs don't mention anything related to that, so I just updated the installation.md docs with the new configuration value.

timvandijck commented 1 week ago

Hi @devajmeireles I'm sorry it took a while to come back on this PR. We had a little discussion in our team and agreed that we will not be merging the PR.

While we acknowledge the benefits of ULID we believe it's better to keep simplicity in mind and not offer too many options. Users can still extend the Media model to modify the current implementation.