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

Search for the migrations table in the correct schema #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marton78
Copy link

@marton78 marton78 commented Apr 29, 2020

Currently, when checking if the migrations table has to be created, doesTableExist() will succeed if there a table named migrations can be found in any schema. This means that postgres-migrations fails to start if there is another table called migrations, even if it's in another schema!

In our case, this lead to an incompatibility with graphile-worker, which has its own migrations table in its own schema.

The fix ensures that only the public schema is considered when looking for the table.

@marton78 marton78 changed the title s Search for the migrations table in the correct schema Apr 29, 2020
@ThomWright
Copy link
Owner

See #33

@marton78
Copy link
Author

I see, same problem, different solution. But that one requires tests and there doesn't seem to be progress since January, whereas this fix "just works". Doesn't it?

@ThomWright
Copy link
Owner

No. This approach was suggested in the thread I linked to, but was thought to be a breaking change:

However it could break things for users who have their migrations table in a schema other than public, which is already possible with the current version of postgres-migrations.

For example if you revoke access to public from your migration user:

REVOKE ALL ON SCHEMA public FROM username;

Then postgres-migrations would create the migrations table in the username schema, and this would work just fine.

To which I responded:

I guess if we add the schema as a user option, we'd have to default to omitting the schema and leaving it implicit.

^ This is what I will accept a PR for. With appropriate tests.

To make sure this keeps working for people, tests for the following would be good:

  • using another schema for the migrations table
  • backwards compatibility: revoke access to public, then run migrations

@marton78
Copy link
Author

marton78 commented May 1, 2020

Sorry, I wasn't aware that it's possible to have the migrations table anywhere else but in the public schema. Is this documented anywhere? Searching for "schema" in this repo reveals only rejected PRs and open issues...

@ThomWright
Copy link
Owner

I also didn't know that! This is why I linked to #33 - it's in there.

See here for some documentation: https://www.postgresql.org/docs/9.6/ddl-schemas.html#DDL-SCHEMAS-PATH

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.

2 participants