-
Notifications
You must be signed in to change notification settings - Fork 60
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
Reduce column size to fit utf8mb4 index #87
base: 3.x
Are you sure you want to change the base?
Conversation
I think Doctrine should be the one to manage length of such fields, @Nyholm can you validate that we can just remove the |
It is a huge BC break to reduce the size of the field. We can not merge the PR as it is. For the record: I believe Symfony best practice for 3rd party bundles states that we should not use annotations when defining the database mapping. If we use XML or Yaml one could override our mapping with their own. |
Let me check, that'd be an ideal solution. @Nyholm I don't believe that's the case:
– http://symfony.com/doc/current/cookbook/bundles/override.html |
Hm, interesting. I thought one could just override the mapping configuration file like you normally would. I'll have to investigate in that further then.. Thank you for the resource. |
Unlucky, Doctrine doesn't handle it correctly. I will open a ticket with them, but doubt they will want to move away from a simple default value. |
I think it's more of an issue with Symfony. Symfony should make sure to use the use the config file in |
Also had problems with overriding the length of a field in another bundle, which is not possible because the configs are not merged. See craue/CraueConfigBundle#20 |
I know we're already at 2.0.0-beta4 so this might be too late. But generally, we could sneak a breaking change into this major release? |
Ping @Nyholm breaking change in 2.0.0? |
1349187
to
853c6d3
Compare
As you set an index on the
name
andownerId
properties of your Entity, creation of the tables fails in an utf8mb4 database:The reason can be found in the MySQL 5.7 upgrade notes:
This pull request reduces the column sizes to support utf8mb4.