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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

postgres: switch from lib/pq to jackc/pgx #382

Merged
merged 4 commits into from
Jul 10, 2022
Merged

postgres: switch from lib/pq to jackc/pgx #382

merged 4 commits into from
Jul 10, 2022

Conversation

craigpastro
Copy link
Contributor

馃憢 Hi there! Closes #339.

It was quite a bit simpler than I imagined, so I hope it is done correctly. Please let me know if you have any suggestions.

Thanks!

tests/e2e/main_test.go Outdated Show resolved Hide resolved
@mfridman
Copy link
Collaborator

mfridman commented Jul 9, 2022

Great, only one change for the CLI in cmd/goose/main.go below line 95.

Need to special case postgres and use pgx as the driver name.

if driver == "postgres" {
	driver = "pgx"
}

Example:

goose/cmd/goose/main.go

Lines 93 to 95 in 1311a0e

if driver == "sqlite3" {
driver = "sqlite"
}

CLI consumers are likely using GOOSE_DRIVER=postgres, so we avoid breaking them. It's an implementation detail that we use pgx.

@craigpastro
Copy link
Contributor Author

Ah, that's a great point. Sorry I missed that.

Updated to add the if statement, and rearranged the comment. A switch statement could also be nice there. It sort of ensures that the comment applies to the statement. WDYT?

@mfridman
Copy link
Collaborator

Ah, that's a great point. Sorry I missed that.

Updated to add the if statement, and rearranged the comment. A switch statement could also be nice there. It sort of ensures that the comment applies to the statement. WDYT?

Ye no worries. Switch statement sounds good to me.

@mfridman
Copy link
Collaborator

Thanks for your contribution 馃帀

@mfridman mfridman merged commit bc72e78 into pressly:master Jul 10, 2022
@craigpastro
Copy link
Contributor Author

It's my pleasure! Thanks for the help.

@craigpastro craigpastro deleted the switch-to-pgx branch July 10, 2022 03:26
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.

Switch from lib/pq to jackc/pgx
2 participants