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

Implement custom migration table name #48

Closed
wants to merge 3 commits into from
Closed

Implement custom migration table name #48

wants to merge 3 commits into from

Conversation

djgrant
Copy link

@djgrant djgrant commented Jun 29, 2020

This enables users to pass their own migration table name.

Migrate can now be called like:

await migrate(dbConfig, "path/to/migration/files", {
  migrationTableName: "schema.my_migrations",
})

I didn't manage to get the tests running locally so they may need looking at.

Let me know if you want any changes!

migrationTableName: string,
) {
const result = await client.query(SQL`
SELECT to_regclass(${migrationTableName}) as matching_tables
Copy link
Author

@djgrant djgrant Jun 29, 2020

Choose a reason for hiding this comment

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

I changed to to_reclass as the original query would check if a table name exists in any schema. This meant migrations would broke in the case that a user specifies a schema in migrationTableName.

@ThomWright
Copy link
Owner

Have you looked through the existing issues/PRs around this?

Search: https://github.com/ThomWright/postgres-migrations/issues?q=migration+table+name

Does this solve the problem of the initial migration hash changing, which is a breaking change for anyone using this library?

And does it work for existing users who have access to the public schema revoked? #33

@djgrant
Copy link
Author

djgrant commented Jun 29, 2020

Published as https://www.npmjs.com/package/@djgrant/postgres-migrations/v/4.1.0-rc.0

@djgrant
Copy link
Author

djgrant commented Jun 29, 2020

Hey @ThomWright, thanks for taking a look at this! I've added a solution for the initial migration hashing problem. It's an awkward problem - I've done the best I can with it.

My feeling is that because that migration is essentially a unique value distributed across users' DBs it makes sense to almost hardcode it. I've moved all the code for creating the initial migration into one module to hopefully make this all a bit more explicit - and be clear that the initial migration is the only migration the library will ever produce itself. I've also added a regression test to make sure that future versions don't break the initial migration.

On #33, I'd need to look into this in more detail another time. Unless you have a solution in mind already?

Published changes as https://www.npmjs.com/package/@djgrant/postgres-migrations/v/4.1.0-rc.1

@djgrant
Copy link
Author

djgrant commented Jun 29, 2020

Worth mentioning that with this implementation a user who changes the location of their migrations table will not be able to simply move the old migrations table.

await migrateWithCustomMigrationTable()
})

test("with custom migration table name in a custom schema", async (t) => {
Copy link
Author

Choose a reason for hiding this comment

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

Does this test satisfy what you were asking for in #33 (comment)?

@djgrant
Copy link
Author

djgrant commented Jul 13, 2020

@ThomWright, did you have any thoughts on this? Happy to close if you don't feel it's the right solution for the library but equally can make changes to get it where you want.

Personally, I'm just relying on the change that allows me to run migrations in a different schema (using SET search_path TO schema_name). I wonder if that would cover most use cases and priority should be given to #33.

@ThomWright
Copy link
Owner

Hi @djgrant - sorry for the delay. I just haven't had the time/energy to give this proper focus. I noticed you published your own version, are you OK just using that for now? Hopefully I'll get around to this another time.

@djgrant
Copy link
Author

djgrant commented Jul 13, 2020

Yes, of course – that's what's forking is for after all! Just wanted to check before I switch focus.

@ThomWright ThomWright mentioned this pull request Dec 21, 2020
totakoko added a commit to totakoko/postgres-migrations that referenced this pull request Apr 7, 2022
@djgrant djgrant closed this May 6, 2022
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