doctrine / data-fixtures

Doctrine2 ORM Data Fixtures Extensions
http://www.doctrine-project.org
MIT License
2.77k stars 224 forks source link

ORMPurger generates table names with unnecessary quotings. #338

Closed jgxvx closed 4 years ago

jgxvx commented 4 years ago

We have an entity called Order, which is a reserved keyword. We use MySQL in production and SQLite to run integration tests. We never had any issues using this configuration:

/**
 * @ORM\Entity(repositoryClass="Acme\LeBundle\Repository\OrderRepository")
 * @ORM\Table(schema="acme_shop", name="`order`")
 * @ORM\HasLifecycleCallbacks
 * @ORM\ChangeTrackingPolicy("DEFERRED_EXPLICIT")
 */
class Order
{
    // snip
}

Using the default quoting strategy (Doctrine\ORM\Mapping\DefaultQuoteStrategy) a table name of acme_shop__order is generated for SQLite.

Starting with doctrine/data-fixtures v1.4.1 this project's build fails when trying to purge the schema:

1) Acme\InAppBundle\Tests\Controller\InAppControllerTest::testPurchaseAction
Doctrine\DBAL\Exception\SyntaxErrorException: An exception occurred while executing 'DELETE FROM acme_shop__"order"':

SQLSTATE[HY000]: General error: 1 near ""order"": syntax error

/home/jenkins/workspace/service.shop.mb.pipeline_develop/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractSQLiteDriver.php:59
/home/jenkins/workspace/service.shop.mb.pipeline_develop/vendor/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php:166
/home/jenkins/workspace/service.shop.mb.pipeline_develop/vendor/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php:146
/home/jenkins/workspace/service.shop.mb.pipeline_develop/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:1069
/home/jenkins/workspace/service.shop.mb.pipeline_develop/vendor/doctrine/data-fixtures/lib/Doctrine/Common/DataFixtures/Purger/ORMPurger.php:157
/home/jenkins/workspace/service.shop.mb.pipeline_develop/vendor/doctrine/data-fixtures/lib/Doctrine/Common/DataFixtures/Executor/AbstractExecutor.php:132
/home/jenkins/workspace/service.shop.mb.pipeline_develop/vendor/doctrine/data-fixtures/lib/Doctrine/Common/DataFixtures/Executor/ORMExecutor.php:66
/home/jenkins/workspace/service.shop.mb.pipeline_develop/vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php:237
/home/jenkins/workspace/service.shop.mb.pipeline_develop/var/cache/test/ContainerVxztlms/EntityManager_9a5be93.php:80
/home/jenkins/workspace/service.shop.mb.pipeline_develop/vendor/doctrine/data-fixtures/lib/Doctrine/Common/DataFixtures/Executor/ORMExecutor.php:71
/home/jenkins/workspace/service.shop.mb.pipeline_develop/vendor/acme/web.dev/Test/DataFixtures/DatabasePrimer.php:88
/home/jenkins/workspace/service.shop.mb.pipeline_develop/vendor/acme/web.dev/Test/DataFixtures/FixturesAwareWebTestCase.php:25

Last Working Version 1.4.0

Breaking Version 1.4.1

Breaking Commit 7f309a20f803c490638ffb7e1c48b25c4aeb2f82

Breaking Pull Request

334

Problem Up to v1.4.0, ORMPurger::getTableName() would use the configured quoting strategy to generate the table name. Which seems like the sensible thing to do. The method would return the correct table name acme_shop__order. The string "acme_shop__order" is not an SQLite keyword.

Starting with v1.4.1, ORMPurger::getTableName() disregards any and all configured strategies and calls getQuotedName() on an instance of Doctrine\DBAL\Schema\Identifier, returning a table name of acme_shop."order", which may or may not be a valid SQLite table name, but is not the same as the one created by the strategy.

This table name is later passed to SqlitePlatform::getTruncateTableSQL() which replaces the . with a __, resulting in the final table name acme_shop__"order" which is used in the executed SQL statement.

Output of schema tool:

$ bin/console --env=test doctrine:schema:create --dump-sql

...snip...

CREATE TABLE "acme_shop__order" (id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, created DATETIME NOT NULL, updated DATETIME NOT NULL, reference VARCHAR(40) NOT NULL, user_id INTEGER DEFAULT NULL, state VARCHAR(30) NOT NULL, context VARCHAR(30) NOT NULL, provider VARCHAR(30) NOT NULL, total NUMERIC(10, 2) NOT NULL, vat NUMERIC(2, 1) NOT NULL, authentication_id INTEGER UNSIGNED DEFAULT NULL, transaction_id INTEGER UNSIGNED DEFAULT NULL);

Expected Purger generates same table names as schema tool.

Actual Purger (in cooperation with SqlitePlatform) generates invalid table names, containing unnecessary quotings, leading to errors when trying to purge data in previously created schema. This behaviour breaks existing code when moving from doctrine/data-fixtures 1.4.0 to 1.4.1.

greg0ire commented 4 years ago

@stephen-lewis can you please have a look at this?

stephen-lewis commented 4 years ago

Apologies @jgxvx I didn't look deeply enough on this originally.

@greg0ire: PR created to fix; essentially it reverts the changes to getTableName and adds a function like getTruncateTableSQL to get the delete SQL statement. I've mimicked the code in orm's SchemaTool.

--edit: grammar

jgxvx commented 4 years ago

Thank you, @stephen-lewis!

greg0ire commented 4 years ago

And thank you @jgxvx , I rarely see such precise bug reports!

greg0ire commented 4 years ago

Can anyone experiencing this please test #340 ? Instructions are here: https://github.com/doctrine/data-fixtures/pull/340#issuecomment-574851931