Use executeUpdate instead of executeQuery for write operation

[goetas]

Q	A
Type	improvement
BC Break	no
Fixed issues

[greg0ire]

Woops, I rebased the wrong PR… does not look like it changed anything though…

EDIT: they're the same code, that's why 😅

[alcaeus]

Thanks @goetas!

[lyrixx]

Hello.

Sorry, I'm super late on this one, but It breaks my application.

The error:

# bin/console --env=test doc:data:drop --force && bin/console --env=test doc:data:crea  && bin/console --env=test doc:mi:mi -n

[...]

     -> REPAIR TABLE export
     -> OPTIMIZE TABLE export
Migration 20180223153152 failed during Execution. Error An exception occurred while executing 'OPTIMIZE TABLE export':

SQLSTATE[HY000]: General error: 2014 Cannot execute queries while other unbuffered queries are active.  Consider using PDOStatement::fetchAll().  Alternatively, if your code is only ever going to run against mysql, you may enable query buffering by setting the PDO::MYSQL_ATTR_USE_BUFFERED_QUERY attribute.

In AbstractMySQLDriver.php line 128:
                                                                                                                                                                                             
  An exception occurred while executing 'OPTIMIZE TABLE export':                                                                                                                             
                                                                                                                                                                                             
  SQLSTATE[HY000]: General error: 2014 Cannot execute queries while other unbuffered queries are active.  Consider using PDOStatement::fetchAll().  Alternatively, if your code is only eve  
  r going to run against mysql, you may enable query buffering by setting the PDO::MYSQL_ATTR_USE_BUFFERED_QUERY attribute.                                                                  
                                                                                                                                                                                             

In Exception.php line 18:
                                                                                                                                                                                             
  SQLSTATE[HY000]: General error: 2014 Cannot execute queries while other unbuffered queries are active.  Consider using PDOStatement::fetchAll().  Alternatively, if your code is only eve  
  r going to run against mysql, you may enable query buffering by setting the PDO::MYSQL_ATTR_USE_BUFFERED_QUERY attribute.                                                                  
                                                                                                                                                                                             

In PDOConnection.php line 52:
                                                                                                                                                                                             
  SQLSTATE[HY000]: General error: 2014 Cannot execute queries while other unbuffered queries are active.  Consider using PDOStatement::fetchAll().  Alternatively, if your code is only eve  
  r going to run against mysql, you may enable query buffering by setting the PDO::MYSQL_ATTR_USE_BUFFERED_QUERY attribute.                                                                  

Then, the executeUpdate method used in this PR is deprecated

So I simply propose to revert this PR since it does not improve (as state in the PR description) nothing (I don't want to be harsh!)

I'm gonna do a PR

[stof]

Using executeQuery would break when using the PrimaryReadReplicaConnection, if the migrations are not performed in a transaction (using a transaction also forces using the primary).

[lyrixx]

@stof Ah! I did not know that. But the migrations is already wrapped in a Transaction, isn't? It's the case on PG at least. I don't use so much MySQL, so I don't know

[stof]

h! I did not know that. But the migrations is already wrapped in a Transaction, isn't? It's the case on PG at least.

This is done by default. But there are ways to disable that wrapping in doctrine/migrations (no idea why you would want to do that though, as I remember my pain when dealing with buggy migrations years ago before switching from MySQL to PG)

[lyrixx]

ok, so let's merge my PR :)

[greg0ire]

@lyrixx I'd like to understand what's going on here… is the mysql driver complaining that we are trying to run OPTIMIZE TABLE export while REPAIR TABLE export is still active?

When I read that comment, I think that the right way to justify a revert would be to show that the query that causes the issue is has a different type from the queries described in https://github.com/doctrine/dbal/blob/3622d6d5823ccaba5d768c311aed27383655041f/lib/Doctrine/DBAL/Connection.php#L1419-L1423

I think that's the case for REPAIR: https://dev.mysql.com/doc/refman/8.0/en/repair-table.html

REPAIR TABLE returns a result set with the columns shown in the following table.

If you agree with that explanation, please add it to your commit message, and please add a comment in the code telling people not to change this to executeStatement in the future and why. If you can write a test that uses REPAIR, that would be even better, but I don't know if that is easy to do (I have no idea if there are similar tests)