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

Deprecate not specifying charset #5278

Closed

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Feb 15, 2022

Follow up of #5277

This is similar to an issue we have had on the ORM with identifier
generator strategies: we default to some value of a setting, then
something better comes out, and we get stuck with the deprecated setting
for backward compatibility reasons.

In this case, utf8 has been deprecated for years and should no longer be
used according to the official documentation:
https://dev.mysql.com/doc/refman/8.0/en/charset-unicode-utf8.html

This kind of BC-break might be handled better by things such as Symfony
Recipes because it is OK to do such BC breaks there since they only
affect new projects.

Should we also do the same for collation? I'm not sure, but there seems to be better collations than the default one.

https://dev.mysql.com/blog-archive/new-collations-in-mysql-8-0-0/

This is similar to an issue we have had on the ORM with identifier
generator strategies: we default to some value of a setting, then
something better comes out, and we get stuck with the deprecated setting
for backward compatibility reasons.

In this case, utf8 has been deprecated for years and should no longer be
used according to the official documentation:
https://dev.mysql.com/doc/refman/8.0/en/charset-unicode-utf8.html

This kind of BC-break might be handled better by things such as Symfony
Recipes because it is OK to do such BC breaks there since they only
affect new projects.
@greg0ire greg0ire force-pushed the deprecate-not-specifying-charset branch from 8b610e8 to 4f4edc4 Compare February 15, 2022 18:38
@morozov
Copy link
Member

morozov commented Feb 15, 2022

I don't agree with this change: a Table object represents the user's intent to have a table in the database. The charset and collation parameters are optional because they are not portable and are not required in the CREATE TABLE DDL.

The platform-aware schema comparison logic for MySQL is aware of the default table charset and collation and uses them to compare the explicit and implicit values on the columns. It doesn't do so for the table charset/collation (#4945) which also applies to other table "options" like ENGINE.

'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/5278',
'Omitting the charset option is deprecated, be explicit about it instead. We recommend using "utf8b4"'
);
$options['charset'] = 'utf8';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The charset and collation parameters are optional because they are not portable and are not required in the CREATE TABLE DDL.

@morozov ok, so although this is not the right way to handle the problem, you do admit there is an issue on this line, right? If the charset parameter is optional, why is there a default value here?

Copy link
Member

@morozov morozov Feb 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you do admit there is an issue on this line, right?

Yes, it will be addressed by #4644.

If the charset parameter is optional, why is there a default value here?

I don't know. See #4644 (comment).

@greg0ire greg0ire closed this Feb 16, 2022
@greg0ire greg0ire deleted the deprecate-not-specifying-charset branch February 16, 2022 08:14
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants