This project is archived and is in readonly mode.

#112 ✓resolved
Aslak Hellesøy

[PATCH] Detect duplicate migration names

Reported by Aslak Hellesøy | May 5th, 2008 @ 10:15 AM

In Rails 1.2.3 (and trunk), if two migrations have the same name (but different versions), the latter will shadow the former, resulting in former migrations not to be run.

This patch fixes this for Rails 1.2.3. I realise the patch won't apply to trunk, but it should be simple to tweak to trunk too (in the #migrations method).

Apologies for not including a test - I didn't have time to investigate how to write one for migrations.

Comments and changes to this ticket

  • Steve Purcell

    Steve Purcell May 5th, 2008 @ 02:37 PM

    Good call. You could even just re-use IllegalMigrationNameError here, since it's essentially illegal to reuse a migration name. :)

  • DHH

    DHH May 6th, 2008 @ 08:59 PM

    • State changed from “new” to “resolved”

    We changed the migration names in edge to run off timestamps instead to avoid duplicates.

  • Aslak Hellesøy

    Aslak Hellesøy May 7th, 2008 @ 07:09 AM

    I don't think you have resolved it ;-)

    I have created a new patch in Git:

    The situation where this can happen is rare, but possible:

    ruby script\generate migration chunky
    # commit it
    # days or weeks later
    ruby script\generate migration bacon
    # Hmm, this should be called chunky (overlooking there is already a chunky
    # migration from before). Rename the migration file to (same_timestamp)_chunky.rb 
    # and the migration class to Chunky. There are now two Chunky migrations
    # with different timestamps.
    rake db:migrate
    # This should have thrown an error, but it doesn't.
  • Aslak Hellesøy

    Aslak Hellesøy May 7th, 2008 @ 11:32 AM

    Steve taught me off line how to make a fix on a branch - I'll do that next time!

  • DHH

    DHH May 7th, 2008 @ 03:33 PM

    With auto-incrementing ids, collision was guaranteed. With the timestamp ids, you have to be creating the same migration at the very same second for a collision to happen. That's unlikely enough that we don't need more software to deal with, imo.

  • Aslak Hellesøy

    Aslak Hellesøy May 7th, 2008 @ 03:52 PM

    This is not about ids or timestamps at all. It's about two migrations with identical class names. The class in the latest file will overwrite the class in the former, and the former migration will never run.

    This is possible if a migration is renamed, and Rails will never warn you about it. (I realise migrations should never be renamed, but it happens sometimes with new uncommitted migrations).

    It's likely to happen in projects with many migrations if a developer renames an uncommitted migration after it's generated.

    I'm not going to persist more, just point to the fact that the patch is only 5 lines (plus tests).

  • DHH

    DHH May 7th, 2008 @ 04:14 PM

    • State changed from “resolved” to “open”

    Ah. I thought you were worried about id clashes. Do you have the patch handy? I don't see it attached here.

  • Steve Purcell
  • Aslak Hellesøy

    Aslak Hellesøy May 7th, 2008 @ 09:55 PM

    In my second comment there is a link to a changeset in Git, which you can just merge into the master branch. Here is the link again:

    One of the benefits of Git is that you don't need patch files in an issue tracker anymore. Just pull patches from other people's clones.


  • DHH

    DHH May 7th, 2008 @ 10:09 PM

    Pulling one-off patches is a huge pain in the ass. It's fine for long-running, expansive branches, but for simple fixes it's far too much work. So we continue to use patch files in the issue trackers for anything but stuff like docrails and rack on rails.

  • Aslak Hellesøy

    Aslak Hellesøy May 7th, 2008 @ 10:31 PM

    Yeah, I can see how it might be a PITA for small patches. I'll use attachments for future small patches.

    You may want to add a note about this in the sidebar here at Lighthouse. Something like "Small patches as attachments in Lighthouse - big contributions in Git clones".

    Thanks for taking the time to review this patch - however small it is.

  • Repository

    Repository May 11th, 2008 @ 07:39 PM

    • State changed from “open” to “resolved”

    (from [10fdf44236ea9abfd327fc59d83670d4bcb3e0ca]) Added protection against duplicate migration names (Aslak Hellesøy) [#112 state:resolved]

  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 09:42 PM

    • Tag cleared.

    Automatic cleanup of spam.

  • Jeff Kreeftmeijer

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href=""></a>