add -dry-run flag

[elijahcarrel]

Adds a -dry-run flag which can be used on up, up-to, up-by-one, down, and down-to commands which performs all of the same print statements and error checking, aside from the ones performed as part of actually executing the up or down migrations respectively.

This adds the following utility:

• Can be used in CI/CD pipelines.
• Developers running migrations against a production database can vet a migration command they are about to run before executing it (ensures they have gotten their flags correct, etc).

This change is fully backwards-compatible both for go clients as well as command-line clients, as it simply adds a new command-line flag and adds a new go option which defaults to false (as one would expect).

To validate this change, nearly the entire e2e test suite was replicated into a new file dry_run_test.go and tests were modified to perform appropriate dry runs before performing the command for real. In each case, the test validates that the dry run produced the same errors (or lack of errors) as the non-dry run command, but that it does not have any effect on the database. If the maintainers so desire, I could instead port these test cases back into the existing test suite (which would have the benefit of less copied test code, but the downside of making each test longer and more complex).

Fixes #281

[elijahcarrel]

Hey @mfridman! Thanks so much for all your work keeping this repository maintained. I know you're in the middle of several other refactors and sorry to bother, but just wanted to see if you might be willing to take a look at this at some point. Thanks in advance!

[mfridman]

Thanks for contributing a PR, I'll take a closer look by the end of the week.

I presume the goose validate command isn't enough, because in addition to parsing you also want to know what migrations would be applied based on the current database state?

[elijahcarrel]

That's exactly it, yes!

[elijahcarrel]

Hey @mfridman, just wanted to bump this once more if you're willing to review. Thanks so much!

[mfridman]

Sorry @elijahcarrel, I've been holding off on reviewing this PR because I'm a bit hesitant about adding more functionality to the /v3 module of this package, with the exception of context support, dependency upgrades and reasonable bug fixes.

My challenge is finding the right balance of adding new features in a way that makes sense with the existing goose API. This project is 10+ yo with many folks contributing, and new Go best practices have evolved over time, and it has been a bit challenging adding new features while compromising ergonomics. The functional options were a means to an end, but I'm not sure how sustainable it is.

Most of my efforts have been concentrated on the /v4 module because it opens the doors to A LOT of interesting functionality.

Having said that, I'll try to find some time to revisit this issue. But at least wanted to give you an update on the delay.

[elijahcarrel]

Makes total sense, thanks so much for your consideration! I'm open to holding off until v4 as it's not urgent (as long as the timeline for v4 isn't years out).

As additional context, the motivation for me is that my company made our own fork years ago (https://github.com/grailbio-external/goose), made several changes, and ceased to keep it up to date with this one, and it's now woefully out of sync . -dry-run is the only important feature present in our fork that isn't already present in this repo, so the motivation here was to rebuild the feature in the upstream/modern goose so that my company can transition back to this upstream goose without having to drop this feature (or make our own fork again, and I don't have confidence we'll do any better at staying up to date this time around…).

[mfridman]

Gotcha, thanks for that explanation. This makes total sense, and it's always appreciated when folks take the time to contribute back upstream.

I need a bit of time to think this through, but I think this is reasonable. Given this touches quite a bit of the internals I'll spend some time reviewing it carefully.

Bonus, this also partially addresses something I've been thinking about previously.

#281

[bmahesh-codimite]

Hi,
Currently I am developing a flow for this.
Here is my idea.
After a every migration i am inspecting the whole db using the following queries.

SELECT table_name FROM information_schema.tables WHERE table_schema = 'public';
SELECT column_name , data_type , is_nullable = 'YES' AS is_nullable , column_default FROM information_schema.columns WHERE table_schema = 'public' AND table_name = 'sample_table' ORDER BY ordinal_position;
SELECT i.relname AS index_name,
a.attname AS column_name,
ix.indisunique AS is_unique
FROM
pg_class AS t,
pg_class AS i,
pg_index AS ix,
pg_attribute AS a
WHERE t.oid = ix.indrelid
AND i.oid = ix.indexrelid AND a.attrelid = t.oid AND a.attnum = ANY(ix.indkey)
AND t.relkind = 'r' AND t.relname = 'sample_table'
ORDER BY i.relname;

Then store the results of these commands in a json and generate a hash for the json and save it inside the migrations folder as a state file (like terraform does). When run the --dry-run command comapre the saved hash with the DB hash and return the results.

@mfridman @elijahcarrel what are you thinking?

if there is any thing to change or incorrect please let me know. :)

(This flow is for the postgresql)