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.

Sign up for GitHub

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

Jump to bottom

Cannot add CREATE TRIGGER statements with the DBAL mysqli driver #1325

Open
mpdude opened this issue Feb 22, 2023 · 0 comments · May be fixed by #1326
Open

Cannot add CREATE TRIGGER statements with the DBAL mysqli driver #1325

mpdude opened this issue Feb 22, 2023 · 0 comments · May be fixed by #1326

Comments

@mpdude
Copy link
Contributor

mpdude commented Feb 22, 2023 •
edited
Loading

Bug Report

Q A
BC Break guess no
Version 3.5.5

More specifically,

  • doctrine/migrations 3.5.5
  • doctrine/dbal 3.6.0
  • PHP 8.1.6
  • ext-mysqli with client API library version "mysqlnd 8.1.16"

Summary

Passing a CREATE TRIGGER ... statement to $this->addSql() in a migration fails when the SQL is executed.

Current behavior

The migration fails with the error message:

An exception occurred while executing a query: This command is not supported in the prepared statement protocol yet

How to reproduce

Create a migration:

class Version... { ...
    public function up(Schema $schema): void
    {
        $this->addSql('CREATE TRIGGER some_trigger BEFORE UPDATE ON some_table FOR EACH ROW UPDATE other_table SET some_column = NEW.other_column WHERE fk = NEW.id');
    }
}

Then, execute the migration using the mysqli driver.

Expected behavior

Migration being executed with no error, trigger created.

Additional information

The DbalExecutor calls Connection::executeQuery():

migrations/lib/Doctrine/Migrations/Version/DbalExecutor.php

Lines 293 to 298 in dc441bb

foreach ($this->sql as $key => $query) {
$this->outputSqlQuery($query, $configuration);
$stopwatchEvent = $this->stopwatch->start('query');
// executeQuery() must be used here because $query might return a result set, for instance REPAIR does
$this->connection->executeQuery($query->getStatement(), $query->getParameters(), $query->getTypes());

This, in turn, will call \Doctrine\DBAL\Driver\Connection::query() (on the DBAL Driver/Connection).

https://github.com/doctrine/dbal/blob/4173dfc23de02e2ac0c99cfb16fcd8af5fedc23b/src/Driver/Mysqli/Connection.php#L68-L71

In \Doctrine\DBAL\Driver\Mysqli\Connection::prepare(), we ultimately get to \mysqli::prepare(), which is where the initial exception is thrown.

Things work when I use Connection::executeStatement() on the DBAL connection, since that will use \Doctrine\DBAL\Driver\Connection::exec() on the DBAL Driver/Connection, at least when no parameters need to be bound.

executeStatement also seems more appropriate, at least for trigger creation. The code snippet quoted above includes a comment why executeQuery() is called, but in the Git history I can only trace that back to a reverted use of executeUpdate(); so basically it has been executeQuery() since a long time.

#1326 adds a new parameter to addSql() to indicate that the statement should be executed with executeStatement(), which would solve the issue.
@mpdude mpdude changed the title Problem with CREATE TRIGGER statement and dry-run mode Problem with CREATE TRIGGER statements and mysqli Feb 22, 2023
@mpdude mpdude changed the title Problem with CREATE TRIGGER statements and mysqli Cannot add CREATE TRIGGER statements with the mysqli driver Feb 23, 2023
@mpdude mpdude changed the title Cannot add CREATE TRIGGER statements with the mysqli driver Cannot add CREATE TRIGGER statements with the DBAL mysqli driver Feb 23, 2023
@mpdude mpdude linked a pull request Feb 23, 2023 that will close this issue
Open
mpdude added a commit to mpdude/doctrine-migrations that referenced this issue Feb 23, 2023
@mpdude
Make it possible to addSql() that is executed as a statement
8b0ac22 
|      Q       |   A
|------------- | -----------
| Type         | improvement
| BC Break     | no
| Fixed issues | fixes doctrine#1325

 #### Summary

When the DBAL connection uses `mysqli` as the driver, prepared statements are sent to the MySQL server through a dedicated protocol mechanism. For example, `CREATE TRIGGER` statements are not possible in this case.

To use SQL statements like `CREATE TRIGGER`, the `DbalExecutor` may not (at least in the case of the `mysqli` driver) use `Connection::executeQuery()`, but has to call `Connection::executeStatement()`. See doctrine#1325 for more details.

This PR adds a new `executeAsStatement` parameter to `\Doctrine\Migrations\AbstractMigration::addSql()`, which defaults to `false` (current behaviour). By setting it to true, a migration can pass the information to the `DbalExecutor` that the statement must be executed with `executeStatement()`, not `executeQuery()`.
mpdude added a commit to mpdude/doctrine-migrations that referenced this issue Aug 16, 2023
@mpdude
Make it possible to addSql() that is executed as a statement
8a454d8 
|      Q       |   A
|------------- | -----------
| Type         | improvement
| BC Break     | no
| Fixed issues | fixes doctrine#1325

 #### Summary

When the DBAL connection uses `mysqli` as the driver, prepared statements are sent to the MySQL server through a dedicated protocol mechanism. For example, `CREATE TRIGGER` statements are not possible in this case.

To use SQL statements like `CREATE TRIGGER`, the `DbalExecutor` may not (at least in the case of the `mysqli` driver) use `Connection::executeQuery()`, but has to call `Connection::executeStatement()`. See doctrine#1325 for more details.

This PR adds a new `executeAsStatement` parameter to `\Doctrine\Migrations\AbstractMigration::addSql()`, which defaults to `false` (current behaviour). By setting it to true, a migration can pass the information to the `DbalExecutor` that the statement must be executed with `executeStatement()`, not `executeQuery()`.
mpdude added a commit to mpdude/doctrine-migrations that referenced this issue Aug 16, 2023
@mpdude
Make it possible to addSql() that is executed as a statement
607c2fa 
|      Q       |   A
|------------- | -----------
| Type         | improvement
| BC Break     | no
| Fixed issues | fixes doctrine#1325

 #### Summary

When the DBAL connection uses `mysqli` as the driver, prepared statements are sent to the MySQL server through a dedicated protocol mechanism. For example, `CREATE TRIGGER` statements are not possible in this case.

To use SQL statements like `CREATE TRIGGER`, the `DbalExecutor` may not (at least in the case of the `mysqli` driver) use `Connection::executeQuery()`, but has to call `Connection::executeStatement()`. See doctrine#1325 for more details.

This PR adds a new `executeAsStatement` parameter to `\Doctrine\Migrations\AbstractMigration::addSql()`, which defaults to `false` (current behaviour). By setting it to true, a migration can pass the information to the `DbalExecutor` that the statement must be executed with `executeStatement()`, not `executeQuery()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make it possible to addSql() that is executed as a statement mpdude/doctrine-migrations
1 participant
@mpdude