search

doctrine / data-fixtures

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

Make the behaviour of DELETE and TRUNCATE purge methods consistent #259

Closed bugadani closed 7 years ago

bugadani commented 7 years ago

Currently, the ORMPurger behaves differently when the DELETE method is used instead of TRUNCATE. TRUNCATE results in a call that quotes keywords in table names, while DELETE does not. This PR adds the same call to the DELETE branch to make them behave the same.

stof commented 7 years ago

Forcing adding backticks is bad:

So this must use the appropriate DBAL APIs instead.

stof commented 7 years ago

Btw, $this->getTableName($class, $platform) already handles quoting, if you configured quoting properly in your entity. So the appropriate DBAL APIs are already used.

bugadani commented 7 years ago

The truncate branch calls $platform->getTruncateTableSQL which in turn calls getQuotedName on the table name, which forces ticks around (at least) keywords, like in my case, 'order'. getTableName does quote in some cases, but it doesn't force quotes around keyword table names, which is not helpful in my case. So while the DBAL does indeed do quoting based on configuration, the truncate branch forces ticks, the delete branch does not. I will update this PR to call getQuotedName, because this should not fail just because my table names are suboptimal.

stof commented 7 years ago

@bugadani it does not force quotes. It applies quotes when the ORM entity is configured to use quoting. Forcing quotes has drawbacks, as said before: it change the case sensitivity of table names when quoting, which might have weird side effects if you quote it here but not when using the ORM.

Instead, make sure your entities are configured to ask for quoting.

stof commented 7 years ago

I'm still -1 on this change. I'm not even sure it will not break things for people configuring quoting properly at the ORM level.

bugadani commented 7 years ago

I understand that and thank you for your input. However, unless TRUNCATE does something differently than DELETE with regards to table name quoting, this should simply align their behaviour. Feel free to reject this PR if it is clearly wrong, but generating syntactically bad queries because of a misconfiguration seems weird to me. This update simply aligns the behaviour of the two branches which I hope does no damage.

Ocramius commented 7 years ago

It applies quotes when the ORM entity is configured to use quoting.

This is done at https://github.com/bugadani/data-fixtures/blob/8c95cc03e7ed9368221fcac7da2f2f79c2b21e97/lib/Doctrine/Common/DataFixtures/Purger/ORMPurger.php#L230

Closing as invalid