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

Feat/pgx pool support #1037

Merged
merged 78 commits into from
Jun 15, 2021
Merged

Conversation

Streppel
Copy link
Contributor

@Streppel Streppel commented May 28, 2021

fixes #472

This PR implements support for pgx/v4 connection pool with their respective API usage, which differs from the standard database/sql. An example of output can be seen here!

The feature gets enabled when pointing out the driver field to pgx/v4, as in the example below:

version: "1"
packages:
  - name: "db"
    path: "internal/db"
    queries: "schema.sql"
    schema: "schema.sql"
    engine: "postgresql"
    driver: "pgx/v4"

If the field is not informed, it defaults to the current behavior.

This wasn't a huge change in terms of implementation, however there were a lot of tests files to be written, so I decided to create separate test files for both the old and new behaviors. That's the reason the number of modified files is quite big.

We've been using it in our projects and everything has been working nicely so far. We would love to get feedback on this and apply any changes if requested.

@brandur
Copy link

brandur commented Jun 4, 2021

Hopefully this isn't too peanut gallery-ish, but just wanted to say: we're pretty heavily invested in the pgx ecosystem right now and we have all the boilerplate that you'd expect from such, so this feature is pretty exciting.

I installed @Streppel's fork, and as an experiment used it to convert a large-ish slice of our pgx-powered string queries to sqlc. So far it's working perfectly — I was able to get us back to a green build and test suite with surprising ease, and thanks to the way DBTX is implemented, we can even interleave existing queries with ones run by sqlc inside the same transaction, which gives us a plausible migration path for bringing in something like sqlc and cutting over to it incrementally as we ported existing calls. Nice work!

@victoraugustolls
Copy link
Contributor

victoraugustolls commented Jun 4, 2021

That's some excellent news! Thanks for sharing this with us, really glad you are liking it so far @brandur !

Pgx is also one of our core libraries at work :)

@kyleconroy
Copy link
Collaborator

This looks great! Hoping to review it in the next few days. I don't foresee many changes before merging this in.

@kyleconroy
Copy link
Collaborator

@Streppel For review purposes, could you split this into two PRs? One with all the code changes and one with all the tests? GitHub is really struggling with the current size.

@kyleconroy kyleconroy merged commit b0508d7 into sqlc-dev:main Jun 15, 2021
@kyleconroy kyleconroy added this to the v1.9.0 milestone Jun 15, 2021
@elvizlai
Copy link

doc driver should be sql_package.

@victoraugustolls
Copy link
Contributor

victoraugustolls commented Aug 14, 2021

@elvizlai in this pr the name was driver, which was changed later here! Also, in the current docs, it is documented as sql_package, you can see it here. If you found a wrong reference, please open an issue to point us in the right direction, thanks!

@coolaj86
Copy link

coolaj86 commented Apr 14, 2022

TL;DR: Working Example

Support was added, this is the format:

sqlc.yaml:

version: 1
packages:
  - path: "tutorial"
    name: "tutorial"
    engine: "postgresql"
    sql_package: "pgx/v4"
    schema: "tutorial-schema.sql"
    queries: "tutorial-queries.sql"
  - path: "reportsv2"
    name: "reportsv2"
    engine: "postgresql"
    sql_package: "pgx/v4"
    schema: "reportsv2-schema.sql"
    queries: "reportsv2-queries.sql"

cc/ @Streppel Would you mind updating your example up top to use the merged behavior?
(I came across this issue before I found the official documentation, so I reason others probably do as well)

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.

Add support for pgx
6 participants