DoctrineEnumBundle icon indicating copy to clipboard operation
DoctrineEnumBundle copied to clipboard

EnumDropCommentCommand does not work on MySQL

Open AlexandruGG opened this issue 3 years ago • 4 comments

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?

AlexandruGG avatar Jun 30 '21 11:06 AlexandruGG

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);

fre5h avatar Jul 30 '21 07:07 fre5h

@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.

AlexandruGG avatar Aug 05 '21 16:08 AlexandruGG

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

craigh avatar Apr 26 '22 13:04 craigh

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.

AlexandruGG avatar Apr 27 '22 07:04 AlexandruGG