Yii2: Document that safeUp() and safeDown() do not work transactionally in migrations for some DBs

[samdark]

See #13827.

There should be only up() and down() left. Transaction should be attempted automatically.

[samdark]

Don't forget to add notes about MySQL behavior to docs and phpdoc.

[SamMousa]

I disagree with this proposal.
Large operations within transactions can be a lot slower. It should be up to the developer if he / she wants to use transactions.

If the issue is with the manual operation then add another command for safe / unsafe migrations.

Automatically using transactions without any knowledge about what is happening in the migration is a big problem.

The bigger problem with this BC break is also the fact that I have to go back and analyze every migration in my history; I then have to check for each of these if they work inside a transaction, and if I want them inside a transaction.

If the issue is friction, reduce that friction, don't remove functionality that people depend upon.
Personally for each migration I consider whether it needs / benefits from a transaction or not.

[samdark]

I'm going to update docs for safe* ones mentioning MySQL and schema changes.

[SilverFire]

Agreed with @SamMousa. Decision on safeness should be on developer

@samdark you know that nobody reads docs. And many developers do not know what it DDL and what is transactional DDL

[SOHELAHMED7]

@SilverFire I disagree. Many developers read the docs and teach themselves. Comprehensive and simple docs is one of the most important foundation for widely accepted libraries/framework/tool. I think each and every point should be included in the doc.

[sergeymakinen]

I’d leave only up() and down() and add a public $useTransaction = true; property to the migration class. It should make API cleaner and you’re still be able to turn transactions off.

[SamMousa]

@sergeymakinen If you change the default then I have to check every migration ever written and decide if it'll work based on an old database state that I can only inspect by restarting from an empty database and running all migrations.

Do not change default behavior for old migration classes please it is one of the few places where I think BC is very important.

If you want new behavior just change the template for new migrations, no need to change the base class.

What would happen with old migration classes that have a safeUp() and safeDown()?? Does it get ignored?

[sergeymakinen]

@SamMousa I was thinking of 2.1/3.0 of course. Such change is trivial. Some find/replace with a regex does the job.

1. In 2.0.12 we add $useTransaction = true and set it to false in up()/down() while deprecating safeUp()/safeDown(). No BC breaks and you still have time to make changes.
2. In 2.1/3.0 we get rid of safeUp()/safeDown().

[SamMousa]

Yes but it means I have to change all migrations. Migrations are not
generally covered by tests..
I don't get why removal is needed. Change the default behavior is fine; but
I have seen no argument for removing the functions... It doesn't improve
readability or usability.

On Mar 31, 2017 11:20 AM, "Sergey Makinen" [e-mail] wrote:

@SamMousa https://github.com/SamMousa I was thinking of 2.1/3.0 of
course. Such change is trivial. Some find/replace with a regex does the job.

1. In 2.0.12 we add $useTransaction = true and set it to false in up()/
   down() while deprecating safeUp()/safeDown(). No BC breaks and you
   still have time to make changes.
2. In 2.1/3.0 we get rid of safeUp()/safeDown().

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/yiisoft/yii2/issues/13831#issuecomment-290662950, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAhYzZ3agNIx0uuKzFRQP-WsO5DkU6Xtks5rrMVRgaJpZM4Ml357
.

[Sarke]

I'd say printing a warning, or maybe go as far as throwing an error, every time there are schema migration changes on a MySQL DB and safeUp is used. Something that makes it clear that schema changes and MySQL are incompatible with safeUp.

Without knowing, the developer will assume that the safeUp is a transaction and "safe", but if it fails their db will be in an inconsistent uncertain state that they will have to fix manually.

Currently it's called "safe" but really isn't.