Add support for pgx

[kyleconroy]

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.

[ldelossa]

We currently use PGX and its batching mechanism. Would be great if you keep pgx batching in mind while this unfolds.

[kyleconroy]

@ldelossa Do you have an example of the batching API? This is the first I've heard of it

[ldelossa]

@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]

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.

[maxhawkins]

Awesome!

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.

[maxhawkins]

* 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.

[mvrhov]

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]

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]

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.

[georgysavva]

Does this mean that you are planning to support pgx native interface i.e. pgxpool?
Or pgx as a driver to database/sql i.e. pgx stdlib

[tv42]

pgx/stdlib works well already.

[kyleconroy]

@georgysavva the plan is to support pgx/stdlib first, and then add support for pgxpool.

[georgysavva]

@kyleconroy Thanks for explaining.
It would be a great improvement to fully support pgx, can't wait for it!

[Streppel]

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

[Streppel]

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

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!

[Streppel]

@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]

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]

@kevinburkemeter pgx/stdlib works ok with sqlc.

[kevinburke1]

working on that. is there a currently supported way to replace pq.Array?

[mvrhov]

afaik you don't need it for standard types.

[kevinburke1]

pq.Array is part of github.com/lib/pq, a competing SQL driver, not part of the standard library, so it seems unlikely pgx would support it

[johanbrandhorst]

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.

[johanbrandhorst]

Did #1037 replace the use of pq.Array for postgres arrays? Otherwise I wouldn't consider this issue completely closed.

[esenmx]

@johanbrandhorst Is it possible to generate pgtype.TextArray? I'm fighting with source codes and sqlc.yaml couldn't find a way yet.

[johanbrandhorst]

I don't think is it, yet.

[johanbrandhorst]

I created #1276 and #1275 to track removal of pq.Array and support of batching, respectively.