Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #5470 (closed defect: untested)

Opened 2 years ago

Last modified 10 months ago

[PATCH] Make migrations use transaction and rollback if something was wrong

Reported by: divoxx@gmail.com Assigned to: rails@bitsweat.net
Priority: normal Milestone: 1.x
Component: ActiveRecord Version: edge
Severity: normal Keywords: verified
Cc: bitsweat, tom@popdog.net, caio@v2studio.com, jay@jay.fm

Description

When a migration is running and an error is raised, everything before the error is applied to the database schema and everything before (obviously) doesn't. Also, the migration version is stored as executed, which creates schema inconsistency and migration version control brokeness.

This patch wraps the migration inside transactions. If something goes wrong, the current migration and all later are cancelled.

Attachments

migration_into_transactions.4485.diff (0.9 kB) - added by divoxx@gmail.com on 06/23/06 02:32:42.
activerecord_ddl_transactions.diff (2.8 kB) - added by adamwiggins on 02/11/07 10:19:17.
Version of original patch which does not disrupt behavior for non-DDL-transactional-capable databases

Change History

06/23/06 02:32:42 changed by divoxx@gmail.com

  • attachment migration_into_transactions.4485.diff added.

06/23/06 04:58:04 changed by bitsweat

  • cc set to bitsweat.
  • version set to 1.1.1.

Please include unit tests demonstrating the correct behavior.

Does it actually work? Some db support DDL in transactions. Most do not.

06/23/06 05:08:43 changed by divoxx@gmail.com

Okay, i'll be adding the unit tests soon. Probably tomorrow. I've tested it with postgresql and worked.

06/25/06 14:47:29 changed by tomafro

  • cc changed from bitsweat to bitsweat, tom@popdog.net.

Postgres and SQL Server support DDL in transactions, but MySQL and Oracle do not. Damon Clinkscales ran a lot of tests to determine this for his talk at RailsConf.

06/25/06 16:56:54 changed by divoxx@gmail.com

I just saw that it should have an "after" instead of "before" up there.

Do you guys think that worth working on a solution for this? Its pretty anoying when that happens. Maybe keeping a stack to control it? Would it be too much for this problem?

07/05/06 02:03:25 changed by david

  • keywords set to unverified.
  • owner changed from David to rails@bitsweat.net.

07/08/06 01:13:14 changed by bitsweat

  • milestone set to 1.x.

Test that it works on the databases which support transactional ddl.

07/23/06 00:50:30 changed by Caio Chassot <caio@v2studio.com>

  • cc changed from bitsweat, tom@popdog.net to bitsweat, tom@popdog.net, caio@v2studio.com.

For databases which do not support transactional ddl, I'd suggest running the down migration when the up fails. If the down migration is written in the same order as the up migration, it should revert the changes that got through.

02/11/07 10:18:03 changed by adamwiggins

  • keywords changed from unverified to verified.
  • version changed from 1.1.1 to edge.

This patch rocks, and should be applied as soon as possible. I have verified that it works on Postgres (DDLs are rolled back), and that it does not adversely affect the behavior on MySQL. The behavior is changed slightly (i.e., inserts/updates are rolled back), but this shouldn't be any problem that I can see. Basically your database is left in an undefined state when some (but not all) of the commands in a given migration fail, and it will most likely require hand-fixing anyway.

That said, I understand that there is concern about any change in behavior for MySQL, by far the most popular database used with Rails apps. Therefore I have expanded the patch to include a supports_dll_transactions? method for adapters, defaulted to false, and defined to true for Postgres. If not set, the migration will not be wrapped in a transaction, and thus no behavior is changed for MySQL and other non-DDL-transaction-capable databases. The message it prints reflects this as well.

I can't stress how critical true transactional integrity on migrations is for certain types of production applications. self.down just won't cut it in these cases. Please apply this patch! :)

02/11/07 10:19:17 changed by adamwiggins

  • attachment activerecord_ddl_transactions.diff added.

Version of original patch which does not disrupt behavior for non-DDL-transactional-capable databases

02/11/07 10:26:49 changed by bitsweat

  • status changed from new to closed.
  • resolution set to untested.

Getting closer! Thanks for pitching in.

08/29/07 01:50:17 changed by jaylev

  • cc changed from bitsweat, tom@popdog.net, caio@v2studio.com to bitsweat, tom@popdog.net, caio@v2studio.com, jay@jay.fm.