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

postgres - Force MigrationsTable in 'public' schema #414

Merged
merged 5 commits into from
Oct 16, 2020
Merged

postgres - Force MigrationsTable in 'public' schema #414

merged 5 commits into from
Oct 16, 2020

Conversation

veger
Copy link
Contributor

@veger veger commented Jul 17, 2020

I ran into troubles when I migrated a new schema in the database:

  • on first run all was ok, and MigrationsTable was stored in public schema (in empty DB this was only available schema)
  • on next run, MigrationsTable was stored in created schema, as it was not available (obviously), the migrations were (tried to) run again, resulting in the dirty flag set in the 'new' MigrationsTable
    I am not entirely sure how this happened, because the 'default schema' is not changed, so it should be/stay be public.

This PR forces the MigrationsTable in public schema.

I chose this approach (and not using SchemaName) to be backwards compatible, if requested I can modify the PR to use SchemaName instead of 'public' for the MigrationsTable location.

@veger
Copy link
Contributor Author

veger commented Jul 17, 2020

TestWithSchema fail because the MigrationsTable location is influenced by search_path.

I wonder what behavior is/should be when search_path is not provided? It should use public schema to store MigrationsTable, right?
So there is an issue (as this is not happening in my cause when I create another schema).
What is desired behavior?

  1. Store MigrationsTable in public schema independent of search_path
  2. Store MigrationsTable in schema of search_path, with a default (when search_path is not provided) in public
    But what happens if schema is created in (first) migration?

Copy link
Member

@dhui dhui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the delayed response.

I think the best course of action is to manage your RDBMS outside of migrate. e.g. don't use migrations to manage your root schema and single root/super user.

Based on the docs, the default postgres search_path is "$user", public. As mentioned by the docs, new tables should be created in the public schema by default:

The first schema in the search path that exists is the default location for creating new objects. That is the reason that by default objects are created in the public schema.

Using the public schema is not the right change since it'll break any users that set the search_path. The postgres db driver is designed to work with search_path.

Can you provide more detail as to what your migrations were doing? More insight as to why migrations failed to run a second time would be helpful.

@veger
Copy link
Contributor Author

veger commented Sep 15, 2020

In an empty (postgres) DB named app, I create a new schema (and some tables in it) using

CREATE SCHEMA IF NOT EXISTS app;

CREATE TABLE app.application (
  id UUID PRIMARY KEY NOT NULL DEFAULT gen_random_uuid(),
  name VARCHAR NOT null,
  -- etc
);

--etc

On first run the app schema is created as well as the table(s) inside the schema. The migrations version table is created in public and shows that the DB is not dirty (migrations all went without a problem).

On second run (nothing should migrate as they were just migrated successfully and I use exactly the same (bash) script to start the whole process), I run into errors:

  • I see that a new migrations version table is created in the app schema. As the old one was (apparently) not found, the migrations are run again.
  • Since app.application table is present already, the migration fails. And the newly created migration version table (in app schema) is is marked dirty and version is set to 0.
  • The migrations version table in the public schema is untouched and still shows the state of the first run.

I am not changing the search_path (as far as I know), but it has the same name as the DB and the role I use to connect to it. Maybe this automatically sets/influences the search_path?

@dhui
Copy link
Member

dhui commented Oct 12, 2020

Using the same schema name as your role/user may be causing the issue since the default search_path is "$user", public.

My guess is that the 2nd (and subsequent) run use the app schema since it was created by the first migration run. The first run probably succeeded since the app schema didn't exist yet.

@veger
Copy link
Contributor Author

veger commented Oct 12, 2020

Makes sense, but it breaks the migrate experience as a subsequent run should work and keep existing schema intact without any change.

If you don't like this solution, would removing $user from the search path being acceptable?

@dhui
Copy link
Member

dhui commented Oct 13, 2020

would removing $user from the search path being acceptable?

migrate shouldn't modify the search_path, but we can update the docs to warn about the various search_path issues

@veger
Copy link
Contributor Author

veger commented Oct 15, 2020

ok, I added an example to the postgres tutorial how to circumvent the issue. But this can only be done when using migrate in the application. When using the (pre-build) tool, it cannot be circumvented...

@coveralls
Copy link

Pull Request Test Coverage Report for Build 891

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 55.639%

Totals Coverage Status
Change from base Build 888: 0.0%
Covered Lines: 3044
Relevant Lines: 5471

💛 - Coveralls

Copy link
Member

@dhui dhui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I added an example to the postgres tutorial how to circumvent the issue. But this can only be done when using migrate in the application. When using the (pre-build) tool, it cannot be circumvented...

Thanks for updating the docs!
I believe you can set the search_path as part of the DB connection string/URL/DSN.

@dhui dhui merged commit f18b165 into golang-migrate:master Oct 16, 2020
@veger veger deleted the fix-postgres-version-table branch October 16, 2020 07:01
@veger
Copy link
Contributor Author

veger commented Oct 16, 2020

Yes you are right, I found out a little later and updated the doc/PR
Hope this will help other in the future to prevent the issue we had

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.

None yet

3 participants