Disable foreign key checks on purge for all mysql versions

croensch commented 2 years ago

For your consideration, see #272. I don't know if the discussion was a yes/no to this feature.

I can push a little refactor of getTruncateTableSQL() to make it fit better.

croensch commented 2 years ago

I initially picked up and rebased this PR to revive or end the discussion around this problem. Now that i looked into it, i understand a few things better:

The ORMPurger touches all tables anyway, so for MySQL where the TRUNCATE is a drop/re-create internally, it should theoretically not be much faster than the current workaround of running the drop & create commands. Still, it would be nice if this feature just works on MySQL too.
In AliceDataFixtures they implemented a fix referencing the original PR. We should not break that, i don't think it does, but it got me thinking... https://github.com/theofidry/AliceDataFixtures/blob/master/src/Bridge/Doctrine/Purger/Purger.php
So i like how DBUnit restores the variables state, perfectly fine pattern to be least-invasive. https://github.com/sebastianbergmann/dbunit/blob/master/src/Operation/Truncate.php
In DBUnit but also this PR i don't like how it's done around every TRUNCATE statement as it may spam the logger. I would rewrite it to call a method (like in DBUnit) before and after the tables loop.
I still think it's a good idea to sniff the connection instead of the platform since only AbstractMySQLDriver is the same since 3.0 while the AbstractMySQLPlatform is only available since DBAL 3.3, but 3.0 is the minimum dependency.

So would you mind if i rewrite it to address all this? In the same PR?

greg0ire commented 2 years ago

DBAL 3.3, but 3.0 is the minimum dependency.

I think it's fine to bump the minimum dependency to 3.3, sniffing the ~connection~ driver (why did you mention the connection?) is not a great idea because of middlewares that can wrap the driver in an arbitrary number of decorators.

So would you mind if i rewrite it to address all this? In the same PR?

Sure, go ahead :slightly_smiling_face:

croensch commented 2 years ago

Sorry for the spam. I got nothing to add or change - it's finished for me.

Regarding the DBAL (Common?) dependency i also check the previous class in a few easy-to-delete lines. Just in case you want to postpone upping the dependency.

greg0ire commented 2 years ago

Reviewers: to be squash-merged

croensch commented 12 months ago

@SenseException @derrabus can we get another look at this?

doctrine / data-fixtures

Disable foreign key checks on purge for all mysql versions #407