Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.x] Dump Postgres migration table from all schemas #52076

Closed
wants to merge 1 commit into from

Conversation

simon-tma
Copy link
Contributor

If a pgsql connection is using a search_path other than public, the current schema:dump command fails because public.migrations doesn't exist. pg_dump doesn't provide a way to set search_path, so just dump the migrations table from all schemas instead.

@hafezdivandari
Copy link
Contributor

Changing 'database.migrations.table' => 'migrations' config value to 'your_schema.migrations' helps?

'table' => 'migrations',

@simon-tma
Copy link
Contributor Author

Only if there's only a single connection used

@hafezdivandari
Copy link
Contributor

pg_dump doesn't provide a way to set search_path

But pg_dump has -n option to declare schema, what am I missing here?

@simon-tma
Copy link
Contributor Author

-n is a filter. It does not set search_path, so the passed table name still only looks in search_path.

@taylorotwell
Copy link
Member

I'm very very scared to change this on a patch release. It almost always breaks existing apps.

@taylorotwell taylorotwell marked this pull request as draft July 10, 2024 19:17
@hafezdivandari
Copy link
Contributor

hafezdivandari commented Jul 10, 2024

I'm sorry but the changes here aren't valid. The hasMigrationTable() respects the search_path but then you want to dump migrations table on every schema. This definitely causes problem!

@simon-tma
Copy link
Contributor Author

I'm sorry but the changes here aren't valid. The hasMigrationTable() respects the search_path but then you want to dump migrations table on every schema. This definitely causes problem!

All tables across all schemas are already dumped, this change just now dumps the data from all migrations tables as well rather than just public.migrations. Please explain how that definitely causes problems.

@hafezdivandari
Copy link
Contributor

  1. One may have declared migrations table with schema name included like 'my_schema.migrations', you are prefixing this with an additional *, so it becomes '*.my_schema.migrations', which is breaking?
  2. We expect only schemas declared on search_path to be dumped right? but you are saying: All tables across all schemas are already dumped, this change just now dumps the data from all migrations tables as well
  3. hasMigrationTable() determines if the public.migrations exists, then you want to dump every migrations table on every schemas?

@simon-tma
Copy link
Contributor Author

simon-tma commented Jul 11, 2024

  1. Corrected
  2. No, all tables are dumped regardless of schema or search_path.
  3. hasMigrationTable() determines if migrations exists within the search_path, then dumps the data from public.migrations which fails if the migrations table is in a different schema. I'm still not sure how changing this to dump the data from migrations tables across all schema will definitely break things

@simon-tma
Copy link
Contributor Author

Updated the patch. It now attempts to detect if a single schema is in use and then dumps only that table. If the migration table already has a schema attached it is left as-is.

@hafezdivandari
Copy link
Contributor

@simon-tma Would you please check PR #52098 to see if it resolves your issue?

@simon-tma
Copy link
Contributor Author

It does not

@hafezdivandari
Copy link
Contributor

@simon-tma would you please share more info? what was the output? What is your search_path and where is your migrations table?

command fails because public.migrations doesn't exist

This shouldn't happen anymore using that PR, because we are checking if the table exists before dumping.

@driesvints
Copy link
Member

Going to close this one now for a lack of activity.

@driesvints driesvints closed this Jul 26, 2024
@simon-tma simon-tma deleted the patch-2 branch February 13, 2025 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants