peterldowns / pgmigrate

a modern Postgres migrations CLI and library
MIT License
70 stars 3 forks source link

pgmigrate doesn't handle 'SET search_path TO...' #4

Closed floitsch closed 1 month ago

floitsch commented 8 months ago

Our migrations contain SET search_path TO ... which drips pgmigrate. Even after changing the table-name to an "absolute" path like public.pgmigrate_migrations it would still not find it:

"ERROR: relation \"pgmigrate_migrations\" does not exist (SQLSTATE 42P01)"
peterldowns commented 8 months ago

Interesting, sorry for the trouble and thanks for the bug report. I am able to reproduce this on my own, and I will fix this.

Out of curiosity, can you share an example migration file and explain a little bit more about the reason you're adjusting the search path within your migrations? I haven't run into this before because I've never done that myself, and I'm curious to learn about this use case.

floitsch commented 8 months ago

Out of curiosity, can you share an example migration file and explain a little bit more about the reason you're adjusting the search path within your migrations?

Of course. Example file: https://github.com/toitware/artemis-releases/blob/aa74252c3c3ee671407a172292743374cdf6934a/broker/supabase/migrations/20230207145057_initial.sql

In our setup we have two database components:

Since our broker is open source (https://github.com/toitware/artemis-releases/tree/main/broker/supabase/migrations) and can be run by our users we have all broker logic in its own schema (toit_artemis) so that our customers can include it without messing up their own DB.

However, some table names, like devices, exist both in the public (billing) schema and the broker. For simplicity, I sometimes just set the search path to toit_artemis in migration files that are only for the broker. This avoids some typing and accidental resolution from the broker to the billing schema.

Fwiw, I'm not a DB expert. There might be better ways to do this.

peterldowns commented 1 month ago

OK, finally revisiting this. I was able to reproduce the issue and I understand the problem. When you have a migration that runs SET search_path TO ..., subsequent pgmigrate commands that do not specify a schema or search path themselves will then execute on the same connection, and run in the new search path. This then breaks.

I've solved this in #7 by using a schema-qualified table name public.pgmigrate_migrations for the default migrations table name, and updating the code to properly handle these schema-qualified table names if you want to set one on your own.

So with this fix, pgmigrate won't break when it's querying against its migration table, even if you keep all the migrations as they are now. Hopefully this is an acceptable fix.

I also tried to do some extra validation that this fix works for you by cloning https://github.com/toitware/artemis-releases/tree/main/broker/supabase/migrations, setting up a supabase project of my own, and running pgmigrate plan / pgmigrate apply / pgmigrate list and confirming that the artemis migrations applied successfully. The only issue I ran into is that I needed to add ?default_query_exec_mode=exec" to the database connection string so that the CLI's postgres library, pgx, can connect to supabase through its pooler. For more information on this, see the pgx documentation — I'll probably add a FAQ entry to the pgmigrate readme about this.

peterldowns commented 1 month ago

Going to close this as soon as the CLI release is updated — if for some reason you're still encountering the issue after upgrading to the new CLI, please let me know and we can re-open and try some other fix.

peterldowns commented 1 month ago

The CLI has been updated and so has the homebrew tap, closing as planned. Thanks again for reporting this bug, definitely expanded my mind a bit :)