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

Fix migrations for redshift (not working out of the box right now) #567

Merged
merged 3 commits into from
Dec 15, 2023

Conversation

vpetrosian
Copy link
Contributor

redshift migrations configured to use pq driver (https://github.com/lib/pq) which is not imported into project. Because of that error is being thrown.
postgres migrations use pgx driver (https://github.com/jackc/pgx)
2 possible solutions here, either import pq or change redshift driver to pgx. This PR is with change from pq to pgx for redshift

db.go Outdated
@@ -16,13 +16,13 @@ func OpenDBWithDriver(driver string, dbstring string) (*sql.DB, error) {
case "mssql":
driver = "sqlserver"
case "redshift":
driver = "postgres"
driver = "pgx"
Copy link
Collaborator

Choose a reason for hiding this comment

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

The alternative is to remove this override entirely, which I think is more correct.

In the CLI we use pgx since we're solely responsible for picking the driver, but in this case a user might be using lib/pq and an override to pgx would fail.

Probably best to just remove it and leave this up to the caller.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Removed in e3dbb9b

I believe this should fix the issue for existing users who want to use lib/pq

Also added an override back to the CLI code to use pgx driver.

vpetrosian and others added 2 commits December 14, 2023 22:40
In order to fix migrations in redshift that do not work out of the box right now
@mfridman mfridman merged commit 6e43ae9 into pressly:master Dec 15, 2023
9 checks passed
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