stripe / pg-schema-diff

Go library for diffing Postgres schemas and generating SQL migrations
MIT License
316 stars 23 forks source link

Make it easier to generate plans from custom commands #32

Closed mortenson closed 1 year ago

mortenson commented 1 year ago

πŸ‘‹ Hi there, really cool project! I just started to build a new web app using sqlc, and tried out generating migrations automatically using this repo: https://gist.github.com/mortenson/c3c1e7f2a1b10c5c3674f8c91123c3e0

The only internal code I copied was getDDLFromPath and openDbWithPgxConfig, so I was wondering if those could be made public, or alternatively (which is why I'm not opening a PR yet), if from looking at my implementation you could come up with a higher level "generate plan" function that takes in similar arguments to the plan command.

Also in my implementation you can see that I'm ignoring some Goose tables, I don't think this needs to be a first-class feature of this library but maybe there's some sort of configurable ignoring that could be done. When I run plan normally I get this error:

$ pg-schema-diff plan --dsn $(grep ^connection_string config.yml | sed "s/.*: //") --schema-dir db/schema
Error: generating plan: validating migration plan: inserting schema in temporary database: executing migration statement: {CREATE TABLE "goose_db_version" (
        "id" integer NOT NULL DEFAULT nextval('goose_db_version_id_seq'::regclass),
        "version_id" bigint NOT NULL,
        "is_applied" boolean NOT NULL,
        "tstamp" timestamp without time zone DEFAULT now()
) 3s []}: ERROR: relation "goose_db_version_id_seq" does not exist (SQLSTATE 42P01) 
diff.Plan{
    Statements: {
        {
            DDL:     "DROP TABLE \"goose_db_version\"",
            Timeout: 1200000000000,
            Hazards: {
                {Type:"DELETES_DATA", Message:"Deletes all rows in the table (and the table itself)"},
            },
        },
    },
    CurrentSchemaHash: "90928d11cb0926a5",
}
bplunkett-stripe commented 1 year ago

Appreciate the feedback πŸŽ‰ ! It's super cool to see other folks wrapping the library for their own purposes !

For now, I'm hesitant to export those functions and expose more API surface than I have to, especially because the library is in its early stages.

In terms of the other issue you're running into I think there's two courses of actions we should take (albeit I can't guarantee a timeline):

  1. Add sequence support. This is already on the roadmap
  2. Add support for the ignore/exclude feature you are requesting. This is not on the roadmap (just added it)-- it seems like a useful feature! I'm not exactly sure how this ignore support would look, since there are different levels of granularity to ignore at. For example, ignoring certain Postgres schemas vs ignoring individual objects of the schemas, like tables and functions.

In terms of feature priority, I think adding support for sequences is more important in the short term to unblock folks. Having ignore support I think is more of a nice-to-have. Let me know what you think!

cc @alexaub-stripe if you have any thoughts here

mortenson commented 1 year ago

While I don't control the schema goose auto-magically applies when running migrations for the first time, I think it's pretty stable and having it included in my schema (thus not included in pg-schema-diff output, pending sequence support) wouldn't be the end of the world.

Definitely focus on features that have the most impact, I'm really excited to see where this library goes!

Feel free to close this issue given that there isn't plans to expose these helper methods, pending more feedback from your teammate. Thanks!

bplunkett-stripe commented 1 year ago

Sweet, sounds like a plan! Good to hear you'll be mostly unblocked come serial support.