gjaldon / ecto_enum

Ecto extension to support enums in models
MIT License
562 stars 131 forks source link

Make `mix do ecto.rollback, ecto.migrate` safe #49

Closed halostatue closed 7 years ago

halostatue commented 7 years ago

When adding a value to an enum type, you can add IF NOT EXISTS to prevent migration failure. This should be the default recommendation for adding a type to an enum.

gjaldon commented 7 years ago

I do not agree. Migrations are supposed to fail when they have already been run in the past especially in production environments. I'd rather it fail noisily than not at all. Thanks for the effort though! 💜

halostatue commented 7 years ago

You may not agree, but psql dump files specify IF NOT EXISTS for just about every object. Your refusal to implement this particular recommendation to documentation is particularly ill-advised, because there is no way to fix the suggested migration because there is no down step possible without going through substantial effort documented elsewhere.

If someone has manually created a type on a database (whether advisable or not), not including IF NOT EXISTS on ALTER TYPE means that you will never be able to get past this particular migration, which leaves you with an undeployable migration.

Ecto migrations prevent a migration from running automatically because after the migration has been added successfully, the migration version number is added to schema_migrations.

Put bluntly, the advice given in the README as it stands is both wrong and dangerous, leading to potentially blocking code.