fre5h / DoctrineEnumBundle

📦 Provides support of ENUM type for Doctrine in Symfony applications.
https://github.com/fre5h/DoctrineEnumBundle
MIT License
459 stars 75 forks source link

EnumDropCommentCommand does not work on MySQL #197

Open AlexandruGG opened 3 years ago

AlexandruGG commented 3 years ago

Hello!

I'm trying the new addition command to drop comments mentioned here: https://github.com/fre5h/DoctrineEnumBundle/blob/main/Resources/docs/hook_for_doctrine_migrations.md#console-command-for-dropping-comments. However, it doesn't look like the SQL generated is valid for MySQL (I'm using 5.7 but I suspect the same holds true for 8.0).

[ERROR] An exception occurred while executing 'COMMENT ON COLUMN table_name.column_name IS ''':

         SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual
         that corresponds to your MySQL server version for the right syntax to use near 'COMMENT ON COLUMN
         table_name.column_name IS ''' at line 1

I can see this SQL is generated by Doctrine here so it's a bit unclear to me why the correct statement is not used. The correct statement would be something on the lines of:

ALTER TABLE table_name MODIFY COLUMN ENUM(...[current values]) NOT NULL COMMENT ''

Is there something wrong in my setup or what could be the issue here?

fre5h commented 3 years ago

Hi. Sorry for the late answer. Looks like it is a bug in Doctrine.

My console command gets platform from the config

  $connection = $this->em->getConnection();

  $platform = $connection->getDatabasePlatform();
  if (!$platform instanceof AbstractPlatform) {
      throw new RuntimeException('Missing database platform for connection.', 3);
  }

So the SQL statement comes from Doctrine

And then

  $sql = $platform->getCommentOnColumnSQL($tableName, $fieldMappingDetails['columnName'], null);
  $connection->executeQuery($sql);
AlexandruGG commented 3 years ago

@fre5h hey, I've looked into this some more.

I can see the statement comes from Doctrine - I also mention this in the description.

The issue here is that I don't think it's correct to use AbstractPlatform::getCommentOnColumnSQL for MySQL. If you look at where it's used you'll notice MySQLPlatform is not in there:

Screenshot 2021-08-05 at 16 59 29

Even though getCommentOnColumnSQL is used for other databases, like in PostgreSqlPlatform, for MySQL it's not used and the protected AbstractPlatform::getColumnComment is used instead: https://github.com/doctrine/dbal/blob/3.1.x/src/Platforms/MySQLPlatform.php#L552.

It's not really your fault, looks like this is just another inconsistency in Doctrine 🤷‍♂️ . But I think we should mention in the documentation that the doctrine:enum:drop-comment command will not work on MySQL.

Maybe the command could also have a check for the platform and fail early with an error message if used with MySQL. If you agree with this I can open a PR.

craigh commented 2 years ago

I just discovered this bug for MariaDB also (which I know is essentially MySQL). but thought it was worth the mention. Any workaround?

AlexandruGG commented 2 years ago

I just discovered this bug for MariaDB also (which I know is essentially MySQL). but thought it was worth the mention. Any workaround?

My workaround was to just write the migration query manually whenever an enum's values needed to change (which should not happen often ideally!).

For example if we have an enum BasketballPositionType: ENUM('PG', 'SG', 'SF', 'PF') and we want to add the C value:

ALTER TABLE players CHANGE position position ENUM('PG', 'SG', 'SF', 'PF', 'C') NOT NULL COMMENT '(DC2Type:BasketballPositionType)'

My other recommendation would be to get on PHP 8.1 and Doctrine >= 2.11 because it supports the new native PHP Enum type: https://www.doctrine-project.org/2022/01/11/orm-2.11.html.