You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Get out the trumpets and ready the 21-gun salute, lib/pq is deprecated (#470). This means it's time to support it's successor, pgx.
This will be the main tracking issue for pgx. It supersedes #28, as I have no intention of adding support for any additional PostgreSQL drivers beyond pgx.
pgx is new to me, so I wanted to spend some time getting the driver working for the example in examples/booktest/postgresql/. Here's a quick diff of that: powersjcb#1
Some learnings:
Several fields in pgtype do not implement MarshalJSON. (at least pgtype.Timestamp and pgtype.VarcharArray) So at first glance, it looks like fields wont be compatible with the emitjsontags feature of sqlc.
Many fields need to be initialized and then have a value Set into them using the following signature (dst *Timestamp) Set(src interface{}) error. User application code will no longer be able to type check these inputs at compile time. For exampleerr := pgtype.Timestamp{}.Set("asdf") will compile without errors.
It would be cool to support decoding composite types at some point. It's pretty tedious to implement DecodeBinary since you have to pull out every value from the pgtype.Record by hand. Since we know the schema that could be automated and save a lot of time.
* Several fields in `pgtype` do not implement `MarshalJSON`. (at least `pgtype.Timestamp` and `pgtype.VarcharArray`) So at first glance, it looks like fields wont be compatible with the `emitjsontags` feature of sqlc.
In the booktest example at least, the available column uses timestamptz whose pgtype counterpart Timestamptz does support JSON. Regardless I'm not sure it's a problem that raw timestamps don't have a JSON encoding. JSON times always have a zone and Postgres timestamps are ambiguous about what zone they're in so there's no natural conversion.
Do you really need to support pgtype... There are a lot of times you don't need it.. and pointer to native type is enough. This also means that you can use the same struct through most application and there is no need to convert between db struct and "domain" struct. The onl conversion is then for viewing purposes.
I noticed we're still using pq.Array for postgres array type (e.g. TEXT[] becomes pq.Array([]string)). Is it possible for these types to map to the pgx types instead (i.e. TEXT[] becomes pgtype.TextArray)? I tried using an override and it didn't seem to work in 1.4.0.
Hey guys! @kyleconroy do we have any updates on this? I've just hit this wall unfortunately.
Edit: I'm creating a fork right now with a colleague and we'll try to work this out for our case (pgxpool.Pool) as we need to deliver something for next week, if we're able to get this fixed I'll let you know here
Reacted by Steve Coffman, Aleksandr Baryshnikov, Mehmet Esen, Mikhail Kalinin, Joe F and omid9h
I ended up doing something that worked out fine for me. You can check what's different in this diff.
Usage remains equal, except that you need to inform a new parameter at the startup configuration yml file, as in
version: "1"packages:
sql_library: "pgx/v4"
If this parameter is missing it defaults to the old behavior. Currently only supporting :one, :many, :exec (still need to implement others commands and tags)
I'm not very fond of this name I've used but couldn't think of anything better at the time. From my restricted testing everything looks fine to me, however I didn't do anything fancy yet. Maybe this could be a start.
Edit: to check it yourself, download my fork, switch branches to feat/pgx_pool_support, compile it and install it locally following the README instructions and test it!
Reacted by Aleksandr Baryshnikov, Mario Carrion, Mikhail Kalinin, Natan Albuquerque, Paul, Wyatt Arent, Kevin Burke, EmilLaursen, Ryan, Alexandru Rosianu and 2 more
Reacted by Andrew Bagshaw, Ryan, Mehmet Esen, Victor and Dan Clipca
@kyleconroy and others: even though the above fits my use case (which is indeed very simple at this moment) it still is very premature in many aspects to even consider opening a pull request. Despite that, do you think the changes made here are heading in the right direction? If so, we could maybe merge this on a new development branch and continue work over there
Just following up here - found another data race in lib/pq and it would be great to have a chance to evaluate other drivers, but at the moment we're pretty tied to using sqlc.
pq.Array implements the database/sql/driver.Valuer and database/sql.Scanner interfaces, so it works fine with pgx in my testing. It's a bit messy to import both, but it works.
Activity
ldelossa commentedon May 28, 2020
We currently use PGX and its batching mechanism. Would be great if you keep pgx batching in mind while this unfolds.
kyleconroy commentedon May 28, 2020
@ldelossa Do you have an example of the batching API? This is the first I've heard of it
ldelossa commentedon May 28, 2020
@kyleconroy we wrap it a bit to handle variable sized input:
https://github.com/quay/claircore/blob/master/pkg/microbatch/microbatch.go
usage:
https://github.com/quay/claircore/blob/master/internal/indexer/postgres/indexpackage.go#L96
powersjcb commentedon Jun 8, 2020
pgx is new to me, so I wanted to spend some time getting the driver working for the example in
examples/booktest/postgresql/. Here's a quick diff of that: powersjcb#1Some learnings:
pgtypedo not implementMarshalJSON. (at leastpgtype.Timestampandpgtype.VarcharArray) So at first glance, it looks like fields wont be compatible with theemitjsontagsfeature of sqlc.(dst *Timestamp) Set(src interface{}) error. User application code will no longer be able to type check these inputs at compile time. For exampleerr := pgtype.Timestamp{}.Set("asdf")will compile without errors.maxhawkins commentedon Jun 9, 2020
Awesome!
It would be cool to support decoding composite types at some point. It's pretty tedious to implement
DecodeBinarysince you have to pull out every value from thepgtype.Recordby hand. Since we know the schema that could be automated and save a lot of time.maxhawkins commentedon Jun 9, 2020
In the booktest example at least, the
availablecolumn usestimestamptzwhosepgtypecounterpartTimestamptzdoes support JSON. Regardless I'm not sure it's a problem that raw timestamps don't have a JSON encoding. JSON times always have a zone and Postgres timestamps are ambiguous about what zone they're in so there's no natural conversion.mvrhov commentedon Jun 9, 2020
Do you really need to support pgtype... There are a lot of times you don't need it.. and pointer to native type is enough. This also means that you can use the same struct through most application and there is no need to convert between db struct and "domain" struct. The onl conversion is then for viewing purposes.
powersjcb commentedon Jun 10, 2020
Thanks for the feedback @mvrhov! I was able to get my test cases working with some native go types and the pgx driver interfaces.
Also, just discovered that sqlc only implements 1-dimensional array types, so that will help keep things simple. 👍
johanbrandhorst commentedon Jul 1, 2020
I noticed we're still using
pq.Arrayfor postgres array type (e.g.TEXT[]becomespq.Array([]string)). Is it possible for these types to map to the pgx types instead (i.e.TEXT[]becomespgtype.TextArray)? I tried using an override and it didn't seem to work in 1.4.0.3 remaining items
georgysavva commentedon Aug 4, 2020
@kyleconroy Thanks for explaining.
It would be a great improvement to fully support pgx, can't wait for it!
Streppel commentedon Dec 23, 2020
Hey guys! @kyleconroy do we have any updates on this? I've just hit this wall unfortunately.
Edit: I'm creating a fork right now with a colleague and we'll try to work this out for our case (
pgxpool.Pool) as we need to deliver something for next week, if we're able to get this fixed I'll let you know hereStreppel commentedon Dec 24, 2020
Hey everyone, just in time for christmas
I ended up doing something that worked out fine for me. You can check what's different in this diff.
Usage remains equal, except that you need to inform a new parameter at the startup configuration yml file, as in
If this parameter is missing it defaults to the old behavior. Currently only supporting :one, :many, :exec (still need to implement others commands and tags)
I'm not very fond of this name I've used but couldn't think of anything better at the time. From my restricted testing everything looks fine to me, however I didn't do anything fancy yet. Maybe this could be a start.
Edit: to check it yourself, download my fork, switch branches to
feat/pgx_pool_support, compile it and install it locally following the README instructions and test it!Streppel commentedon Jan 5, 2021
@kyleconroy and others: even though the above fits my use case (which is indeed very simple at this moment) it still is very premature in many aspects to even consider opening a pull request. Despite that, do you think the changes made here are heading in the right direction? If so, we could maybe merge this on a new development branch and continue work over there
kevinburke1 commentedon Jan 11, 2021
Just following up here - found another data race in lib/pq and it would be great to have a chance to evaluate other drivers, but at the moment we're pretty tied to using sqlc.
tv42 commentedon Jan 11, 2021
@kevinburkemeter pgx/stdlib works ok with sqlc.
kevinburke1 commentedon Jan 12, 2021
working on that. is there a currently supported way to replace
pq.Array?mvrhov commentedon Jan 12, 2021
afaik you don't need it for standard types.
kevinburke1 commentedon Jan 12, 2021
pq.Arrayis part of github.com/lib/pq, a competing SQL driver, not part of the standard library, so it seems unlikely pgx would support itjohanbrandhorst commentedon Jan 12, 2021
pq.Arrayimplements thedatabase/sql/driver.Valueranddatabase/sql.Scannerinterfaces, so it works fine withpgxin my testing. It's a bit messy to import both, but it works.johanbrandhorst commentedon Jun 16, 2021
Did #1037 replace the use of
pq.Arrayfor postgres arrays? Otherwise I wouldn't consider this issue completely closed.esenmx commentedon Aug 31, 2021
@johanbrandhorst Is it possible to generate
pgtype.TextArray? I'm fighting with source codes and sqlc.yaml couldn't find a way yet.johanbrandhorst commentedon Aug 31, 2021
I don't think is it, yet.
johanbrandhorst commentedon Nov 8, 2021
I created #1276 and #1275 to track removal of
pq.Arrayand support of batching, respectively.