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

Migration fails when using SELECT queries in addSql #1004

Closed
bcremer opened this issue Jun 18, 2020 · 7 comments
Closed

Migration fails when using SELECT queries in addSql #1004

bcremer opened this issue Jun 18, 2020 · 7 comments
Milestone

Comments

@bcremer
Copy link

bcremer commented Jun 18, 2020

BC Break Report

Q A
BC Break yes
Version 3.0.0

Summary

When using an SELECT query in addSql the migrations fails with an PDO Driver exception on MySQL 5.7 (PDO::MYSQL_ATTR_USE_BUFFERED_QUERY is enabled).

In AbstractMySQLDriver.php line 106:
                                                                   
  An exception occurred while executing 'SHOW FULL TABLES WHERE Table_type = 'BASE TABLE'':                                       
                                                                   
  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 set  ting the PDO::MYSQL_ATTR_USE_BUFFERED_QUERY attribute.  

Previous behavior

SELECT queries did not raise exception while executing the migration.

Current behavior

Execution results in PDO Driver Exception

How to reproduce

Given the following migration file:

<?php

namespace App\Infrastructure\Migrations;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

final class Version20200618062514 extends AbstractMigration
{
    public function up(Schema $schema) : void
    {
        $this->addSql('SELECT 1');
    }
}

Execute migration on MySQL 5.7 or higher:

php bin/console.php migrations:execute -n  'App\Infrastructure\Migrations\Version20200618062514'   
\Version20200618062514'   
[notice] Executing App\Infrastructure\Migrations\Version20200618062514 up
[error] Migration App\Infrastructure\Migrations\Version20200618062514 failed during Post-Checks. Error: "An exception occurred while executing 'SHOW FULL TABLES WHERE Table_type = 'BASE TABLE'':

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 106:
                                                                                                                                                                            
  An exception occurred while executing 'SHOW FULL TABLES WHERE Table_type = 'BASE TABLE'':                                                                                 
                                                                                                                                                                            
  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 PDOConnection.php line 83:
                                                                                                                                                                            
  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 PDOConnection.php line 78:
                                                                                                                                                                            
  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.                                

Background

We used the $this->addSql('SELECT 1'); to mitigate or hide was executed but did not result in any SQL statements. Warnings for Migrations that changed later on because of external schema changes.

The underlying reason for the failure is the usage of executeUpdate in \Doctrine\Migrations\Version\DbalExecutor::executeResult.

This can be reproduced with the DBAL Connection:

$connection->executeUpdate('SELECT 1');
$connection->executeUpdate('Another Query'); // fails with the above exception

Given this is just a workaround/hack/miss use from our side feel free to close this issue.

This issue was opened to document this behavior so it can be used for later reference.

@bcremer
Copy link
Author

bcremer commented Jun 18, 2020

Found a simple fix for our specific use case:

    public function up(Schema $schema): void
    {
-        $this->addSql('SELECT "some dummy message to mute warnings"');
+        $this->addSql('-- "some dummy message to mute warnings"');
    }

@goetas
Copy link
Member

goetas commented Jun 19, 2020

Found a simple fix for our specific use case:

That is neat!

I have documented this bc break in #1006

@Slamdunk
Copy link
Contributor

Slamdunk commented Nov 27, 2020

This also affects 2.3 version. Everything was fine till in 2.2.1.

@TomHAnderson
Copy link
Member

I saw this same issue recently. If 2.2.1 changed it then what about that release made the change? https://github.com/doctrine/migrations/releases/tag/2.2.1
It was a small release and annotations for the abstract don't stand out to me as the culprit.

The error message gives the solution: "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."

This isn't an error I would expect another DBMS to throw because the PDO attribute is for MySQL. So I don't see the harm in setting it even if you're using another DBMS too.

PDO::setAttribute(PDO::MYSQL_ATTR_USE_BUFFERED_QUERY, true);

Can you try setting that value to see if following the error's suggestion actually works?

@Slamdunk
Copy link
Contributor

If 2.2.1 changed it then what about that release made the change?

I'm sorry I've expressed bad myself: everything worked in 2.2.1, then it stopped working in 2.3.0

PDO::setAttribute(PDO::MYSQL_ATTR_USE_BUFFERED_QUERY, true);

The attribute cannot be set statically.

Can you try setting that value to see if following the error's suggestion actually works?

Even set in the right place, it doesn't work.

By the way, how is this error even possible, if nowhere in the code the connection is set to unbuffered queries?

@stof
Copy link
Member

stof commented Nov 27, 2020

Adding a SELECT 1 query is clearly a hack here, and does not provide any value for a migration (the migration tool won't do anything with the selected result, and the fact that it now uses an API which actually does not fetch them all is precisely the reason it breaks).
If a hack is wanted to remove the was executed without queries warning is wanted, adding a SQL comment is better than adding a SELECT query. So I suggest closing this a won't fix.

@goetas
Copy link
Member

goetas commented Nov 28, 2020

This change has been reverted in #1071, and it is already in the 3.0.x branch, but not yet released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants