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

[2.x] Changed the column type of profile_photo_path #794

Merged
merged 1 commit into from
May 26, 2021

Conversation

rabin-io
Copy link
Contributor

@rabin-io rabin-io commented May 26, 2021

As the TEXT type on some DBs, creates an column with a limit of 2/4 GB.

This should increase compatibly with other DBs, with no impact on the migration it self. But we DO limit the column length, which is a good thing for this column use case. As a good practice URI should not be longer then 2K.

There is no mention of max URI length in the RFC3986, but there are limitation imposed by the software like the browsers them self.

DB typeString typeTinyText typeText typeMediumText typeLongText
MySQL varchar({$column->length}) tinytext text mediumtext longtext
PostgreSQL varchar({$column->length}) varchar(255) text text text
SQLite varchar text text text text
MSSQL varchar({$column->length}) nvarchar(255) nvarchar(max) nvarchar(max) nvarchar(max)
Informix varchar({$column->length}) : length < 255
lvarchar({$column->length}) : length >= 255
text text text text

ref: issue #789

As the TEXT type on some DBs, creates an column with a limit of 2/4 GB.

This should increase compatibly with other DBs, with no impact on the migration it self. But we DO limit the column length, which is a good thing for this column use case. As a good practice URI should not be longer then 2K.

There is no mention of max URI length in the RFC3986, but there are limitation imposed by the software like the browsers them self.
@driesvints driesvints changed the title Changed the column type of profile_photo_path to string(2048) [2.x] Changed the column type of profile_photo_path to string(2048) May 26, 2021
@driesvints driesvints changed the title [2.x] Changed the column type of profile_photo_path to string(2048) [2.x] Changed the column type of profile_photo_path May 26, 2021
@taylorotwell taylorotwell merged commit e97e85d into laravel:2.x May 26, 2021
@rabin-io rabin-io deleted the issue-789 branch May 26, 2021 12:21
@miclaus
Copy link
Contributor

miclaus commented Oct 31, 2021

@rabin-io @taylorotwell
wouldn't storing only the hash (and extension) of the profile photo suffice?

the way that it's setup at the moment, the profile photo path is saved for every entry, which is redundant.

also if the path where profile pictures are stored were to change, entries would also have to be updated.

saving only the hash would only require at most varchar(45), e.g. {hashName}.jpeg where {hashName} would have a length of 40 chars (because hashName).

how about this:

$table->string('profile_photo', 45)->nullable();

...plus all necessary changes resulting therefrom – i could do a PR if you'd want me to.

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

Successfully merging this pull request may close these issues.

3 participants