Migrate: Postgres: migrate drop doesn't drop everything inside database

[dahu33]

Describe the Bug
Drop doesn't drop everything inside database as is supposed to do. With the prostgres driver it just drops the tables leaving behind, materialized views, enums, sequences etc...

Steps to Reproduce
Steps to reproduce the behavior:
1) create a simple migration: echo "CREATE TYPE mood AS ENUM ('sad', 'ok', 'happy');" > 1_mood.up.sql
2) run migrate up; migrate drop; migrate up

Expected Behavior
Drop should fully clean the database so the migration can be reapplied. FlywayDB does this perfectly (see how here).

Migrate Version
v4.2.5

Loaded Source Drivers
Source drivers: file

Loaded Database Drivers
Database drivers: postgres, postgresql, stub

Go Version
go version go1.12.1 darwin/amd64

[dhui]

Do you have down migrations?
If not, please read this

The best way to handle the issue you're facing is to run migrate down instead of migrate drop.
In fact, we should probably remove migrate drop to reduce the confusion.

[dahu33]

Do you have down migrations?
If not, please read this

The best way to handle the issue you're facing is to run migrate down instead of migrate drop.
In fact, we should probably remove migrate drop to reduce the confusion.

Down migrations are great on paper but unfortunately, in real life, they don't work for every case scenarios. Also, they are currently optional and IMO should stay that way.

Concerning the migrate drop command, it's a handy shortcut in a development environment to restart fresh/clean, IMO it's a great feature not a bug so I would rather improve it than remove it.

[dhui]

Down migrations are great on paper but unfortunately, in real life, they don't work for every case scenarios. Also, they are currently optional and IMO should stay that way.

It's strongly recommended that your migrations are idempotent and that you create down migrations for every up migration. You'll save yourself future headaches by doing so.
What's a case where you cannot write a proper down migration?

Concerning the migrate drop command, it's a handy shortcut in a development environment to restart fresh/clean, IMO it's a great feature not a bug so I would rather improve it than remove it.

Feel free to open a PR to improve migrate drop. I don't have plans to remove it, but it's existence/usage seems like an anti-pattern to me. e.g. a big hammer that allows you to avoid careful design

[psigen]

Well, this just bit me as well.

What's a case where you cannot write a proper down migration?

The specific use-case I had was in building developer test tools for down migrations. If the down function I've written is messed up, it is convenient to reset the (small) local test database and up to the version in question again so I can retest. This works well with live-reloading to make iteration very fast.

It seems like drop is not intended for that purpose, so I guess I will use raw psql commands instead?

[dhui]

When a mistake is made for a migration, you can either:

1. Fix the DB manually and force to a previous migration version. This is the recommended approach if there's data you want to keep in the DB
2. Wipe the DB and run up to the last known good migration. drop should work for most migrations but sometimes, migrations manipulate the DB in ways that drop will remove the data. e.g. adding a user to the DB
   Also, do not use this approach if there's data if you want to keep in the DB.

[psigen]

Perhaps I'm not following:

• I am trying to support (a) authoring migrations and (b) tests that validate that migrations are working correctly.
• I am familiar with approaches (1) and (2), and I am using approach (2) because this is for local development and testing and only uses test data.

From the original comment:

Drop doesn't drop everything inside database as is supposed to do. With the prostgres driver it just drops the tables leaving behind, materialized views, enums, sequences etc...

This means that when I try to implement (2) using drop followed by up, it fails because the database contains old enums, etc.

So this seems to imply one of two things:

1. This is not the intended behavior, and drop is supposed to clear these other items. _(make a fix to the code)_
2. This is not a supported use case, and drop should not be expected to work for use case (2). _(make a fix to the docs)_

@dhui do you have some guidance on which of these two options is intended?

[dhui]

The worry with drop is running it on a shared DB. e.g. shared with other apps/users

This is not the intended behavior, and drop is supposed to clear these other items. (make a fix to the code)

I'm leaning towards this approach, provided that drop is scoped to the schema that migrate is managing. e.g. the current schema
This approach assumes that another system/framework is not managing data (e.g. tables, views, enums, sequences, etc) in the same schema. For postgres, it may make sense to remove all objects in the migrate managed namespace using pg_class, so we don't need to worry about specific data types, but I'm not very familiar with the pg system catalogs. We should also update the drop docs accordingly to warn about the dangers.

@psigen For your specific case, it may be easier to run your DB in a container and blow out the volume when a down migration breaks. Anyways, I'm happy to review any PR that safely improves drop.

[yihuang]

drop owned by current_user can work if we don't care about also removing other schemas ;D

[psigen]

This makes sense, I can try and put together a PR to clarify documentation if I get a chance.

I'm already using a container, I was just hoping not to have to manage its lifecycle from within the test harness, since that will requires some DIND tricks. But I'll take that approach instead.

[dhui]

@psigen I'm not familiar with DIND, but you can try using dktest or other alternatives which migrate uses to run automated tests against DBs using docker containers.
Full disclosure, I factored out migrate/testing to dktest