Skip to content

Add support for v5 of the pgx Go driver #1823

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

Closed
2 of 6 tasks
kyleconroy opened this issue Aug 29, 2022 · 24 comments
Closed
2 of 6 tasks

Add support for v5 of the pgx Go driver #1823

kyleconroy opened this issue Aug 29, 2022 · 24 comments
Labels
enhancement New feature or request proposal
Milestone

Comments

@kyleconroy
Copy link
Collaborator

kyleconroy commented Aug 29, 2022

What do you want to change?

This proposal is a work in progress

v5.0.0 of jackc/pgx is coming in September (jackc/pgx#1273). The changes are significant. I think it's a good opportunity to make some backwards-incompatible changes to fix some long standing issues with our pgx support

What issues should be fixed?

What database engines need to be changed?

PostgreSQL

What programming language backends need to be changed?

Go

@kyleconroy kyleconroy added enhancement New feature or request triage New issues that hasn't been reviewed proposal and removed triage New issues that hasn't been reviewed labels Aug 29, 2022
mcdoker18 added a commit to mcdoker18/sqlc that referenced this issue Oct 2, 2022
To support two version of pgx in the e2e tests at the same time we move 'pgx/v4' tests to the separated v4 directory.
Then new 'pgx/v4' e2e tests will be located in the v5 dir.
mcdoker18 added a commit to mcdoker18/sqlc that referenced this issue Oct 2, 2022
* 'pgx/v5' option requires '--experimental' cli flag.
* array, ranges, multiranges uses generic version for 'pgx/v5'. It supports multidemensional arrays.
* 'pgx/v5' implementation prefers use pgtype types. For example, pgtype.Bool instead of sql.NullBool.
* added support for new pg types:
  * datemultirange
  * tsmultirange
  * tstzmultirange
  * nummultirange
  * int4multirange
  * int8multirange
  * bit
  * varbit
  * cid
  * oid
  * tid
  * circle
  * line
  * lseg
  * path
  * point
  * polygon
mcdoker18 added a commit to mcdoker18/sqlc that referenced this issue Oct 2, 2022
mcdoker18 added a commit to mcdoker18/sqlc that referenced this issue Oct 2, 2022
* 'pgx/v5' option requires '--experimental' cli flag.
* array, ranges, multiranges uses generic version for 'pgx/v5'. It supports multidemensional arrays.
* 'pgx/v5' implementation prefers use pgtype types. For example, pgtype.Bool instead of sql.NullBool.
* added support for new pg types:
  * datemultirange
  * tsmultirange
  * tstzmultirange
  * nummultirange
  * int4multirange
  * int8multirange
  * bit
  * varbit
  * cid
  * oid
  * tid
  * circle
  * line
  * lseg
  * path
  * point
  * polygon
mcdoker18 added a commit to mcdoker18/sqlc that referenced this issue Oct 2, 2022
@mcdoker18
Copy link
Contributor

Hi @kyleconroy !

I decided to try to implement the support pgx v5 in the sqlc. I should have discussed the implementation details with you before I started to work. Sorry for this. But It doesn't take too much time, so I opened 3 PRs:

@kyleconroy
Copy link
Collaborator Author

@mcdoker18 wow, thank you! I'm a bit short on time right now, so please excuse the slow review times. I'm hoping to get these reviewed within the next week.

@advdv
Copy link

advdv commented Oct 16, 2022

Looking forward to this!

@brlala
Copy link

brlala commented Oct 17, 2022

hopefully this gets merged

kyleconroy pushed a commit that referenced this issue Oct 21, 2022
To support two version of pgx in the e2e tests at the same time we move 'pgx/v4' tests to the separated v4 directory.
Then new 'pgx/v4' e2e tests will be located in the v5 dir.
@kyleconroy kyleconroy added this to the v1.17.0 milestone Nov 9, 2022
@ekovacs
Copy link

ekovacs commented Nov 9, 2022

this would allow for using things like https://github.com/jackc/pgx/wiki/Numeric-and-decimal-support which requires pgx/v5

akutschera pushed a commit to akutschera/sqlc that referenced this issue Nov 10, 2022
…#1873)

To support two version of pgx in the e2e tests at the same time we move 'pgx/v4' tests to the separated v4 directory.
Then new 'pgx/v4' e2e tests will be located in the v5 dir.
mcdoker18 added a commit to mcdoker18/sqlc that referenced this issue Nov 17, 2022
* 'pgx/v5' option requires '--experimental' cli flag.
* array, ranges, multiranges uses generic version for 'pgx/v5'. It supports multidemensional arrays.
* 'pgx/v5' implementation prefers use pgtype types. For example, pgtype.Bool instead of sql.NullBool.
* added support for new pg types:
  * datemultirange
  * tsmultirange
  * tstzmultirange
  * nummultirange
  * int4multirange
  * int8multirange
  * bit
  * varbit
  * cid
  * oid
  * tid
  * circle
  * line
  * lseg
  * path
  * point
  * polygon
mcdoker18 added a commit to mcdoker18/sqlc that referenced this issue Nov 17, 2022
* 'pgx/v5' option requires '--experimental' cli flag.
* array, ranges, multiranges uses generic version for 'pgx/v5'. It supports multidemensional arrays.
* 'pgx/v5' implementation prefers use pgtype types. For example, pgtype.Bool instead of sql.NullBool.
* added support for new pg types:
  * datemultirange
  * tsmultirange
  * tstzmultirange
  * nummultirange
  * int4multirange
  * int8multirange
  * bit
  * varbit
  * cid
  * oid
  * tid
  * circle
  * line
  * lseg
  * path
  * point
  * polygon
mcdoker18 added a commit to mcdoker18/sqlc that referenced this issue Nov 20, 2022
kyleconroy pushed a commit that referenced this issue Nov 22, 2022
* Added e2e tests for pfx v5 (#1823)

* regenerate e2e using 1.16.0 sqlc version

* added missing pgx v5 tests
@haines
Copy link
Contributor

haines commented Nov 30, 2022

Column override with query alias produces wrong type #1752

I've created a PR with a fix for this one! #1884

@valensto
Copy link

valensto commented Dec 2, 2022

Hi, seems merged but not working for me when I use pgx/v5

When I try to use pgx/v5 to sqlc.yaml

engine: "postgresql"
  gen:
    go:
      package: "postgres"
      sql_package: "pgx/v5"

The output file use database/sql as package

import (
    "context"
    "database/sql"
)

type DBTX interface {
    ExecContext(context.Context, string, ...interface{}) (sql.Result, error)
    PrepareContext(context.Context, string) (*sql.Stmt, error)
    QueryContext(context.Context, string, ...interface{}) (*sql.Rows, error)
    QueryRowContext(context.Context, string, ...interface{}) *sql.Row
}

But work perfectly with pgx/v4 with the same configuration.

import (
	"context"

	"github.com/jackc/pgconn"
	"github.com/jackc/pgx/v4"
)

I tried to use both homebrew installation and docker use to generate and both generate the same thing.

I just saw 'pgx/v5' golang sql package requires enabled '--experimental'
And try to use it with

@docker run --rm -v $$(pwd):/src -w /src/tools/sqlc kjconroy/sqlc:latest --experimental generate

or

sqlc --experimental generate

But no success

I check in the lastest version of docker and saw support for pgx/v5 but not seem to work

Did I miss something?

@mcdoker18
Copy link
Contributor

Did I miss something?

kjconroy/sqlc:latest docker image uses the latest release of sqlc 1.16.0 which doesn't include the pgx/v5 support. You can build the main branch from source.

https://github.com/kyleconroy/sqlc/blob/main/docs/guides/development.md#building

@valensto
Copy link

valensto commented Dec 3, 2022

Ok thank @mcdoker18 ! Maybe add it to the documentation? Because start with sqlc 2 days ago and really believed it was already included.

Do you know when it's gonna be port to docker image? 😊

Thank for you work with v5 btw 👌

@mcdoker18
Copy link
Contributor

Do you know when it's gonna be port to docker image? 😊

@kyleconroy decides when new release will be available.

@dht-hedaihua
Copy link

Did I miss something?

kjconroy/sqlc:latest docker image uses the latest release of sqlc 1.16.0 which doesn't include the pgx/v5 support. You can build the main branch from source.

https://github.com/kyleconroy/sqlc/blob/main/docs/guides/development.md#building

thank @mcdoker18 , I successfully solved it according to the above!!! However, it is really recommended to update the document and release the corresponding version in a timely manner, so as not to have to go to the issue to find the answer.

@dht-hedaihua
Copy link

Hi,I encountered a type conversion or search execution problem when using sqlc2 and pgx/v5.See if any experts can help me:
The problem is that I used the following sql script to generate a simple search method:

-- name: SearchExamTemplatesByNameOrGradeSubjectOrCreatorName :many
SELECT *
FROM noquestion_content_exam_template 
WHERE (template_name LIKE ANY(@search_key_words::text[]))
OR (grade_subject LIKE ANY(@search_key_words::text[]))
OR (creator_name LIKE ANY(@search_key_words::text[]))
ORDER BY updated_at DESC
LIMIT $1
OFFSET $2;

Then the go method generated by sqlc is as follows:

const searchExamTemplatesByNameOrGradeSubjectOrCreatorName = `-- name: SearchExamTemplatesByNameOrGradeSubjectOrCreatorName :many
SELECT id, template_name, grade_subject, question_total_num, creator_name, created_at, updated_at
FROM noquestion_content_exam_template 
WHERE (template_name LIKE ANY($3::text[]))
OR (grade_subject LIKE ANY($3::text[]))
OR (creator_name LIKE ANY($3::text[]))
ORDER BY updated_at DESC
LIMIT $1
OFFSET $2
`

type SearchExamTemplatesByNameOrGradeSubjectOrCreatorNameParams struct {
	Limit          int32
	Offset         int32
	SearchKeyWords pgtype.Array[string]
}

func (q *Queries) SearchExamTemplatesByNameOrGradeSubjectOrCreatorName(ctx context.Context, arg SearchExamTemplatesByNameOrGradeSubjectOrCreatorNameParams) ([]NoquestionContentExamTemplate, error) {
	rows, err := q.db.Query(ctx, searchExamTemplatesByNameOrGradeSubjectOrCreatorName, arg.Limit, arg.Offset, arg.SearchKeyWords)
	if err != nil {
		return nil, err
	}
	defer rows.Close()
	items := []NoquestionContentExamTemplate{}
	for rows.Next() {
		var i NoquestionContentExamTemplate
		if err := rows.Scan(
			&i.ID,
			&i.TemplateName,
			&i.GradeSubject,
			&i.QuestionTotalNum,
			&i.CreatorName,
			&i.CreatedAt,
			&i.UpdatedAt,
		); err != nil {
			return nil, err
		}
		items = append(items, i)
	}
	if err := rows.Err(); err != nil {
		return nil, err
	}
	return items, nil
}

The key now is to generate the parameter SearchKeyWords of pgtype. Array[string] type in the upper part.After I assigned the parameter in the format, I found that the structure searched by using the search method passed by the parameter was empty. However, if I change the type of this parameter to the pgtype.FlatArray[string], everything will be OK. I don't know what caused this?? But now sqlc always converts text[] type to pgtype.Array[String] type. I try to use the override parameter in sqlc.yaml to make the following configuration to force the change of its conversion rules:

 overrides: 
       - db_type: "pg_catalog.text[]"
          go_type: 
               import: "github.com/jackc/pgx/v5"
  	       package: "pgtype"
	       type: "FlatArray"

When I reexecuted sqlc generate --experimental, no error was reported, but it was found that the generated SearchKeyWords parameter was still of the pgtype. Array[string] type, and the pgtype. FlatArray[string] type was not generated as expected.

Did I miss something? Thanks anybody ~

@Davincible
Copy link

In my models.go netip doesn't get imported in the generated code when using the inet type in a table

@alicebob
Copy link

Hi!
do I need to do anything special to use a pgxpool? I currently get this:

./main.go:137:29: cannot use db (variable of type *"github.com/jackc/pgx/v5/pgxpool".Pool) as type sqlc.DBTX in argument to sqlc.New:
	*"github.com/jackc/pgx/v5/pgxpool".Pool does not implement sqlc.DBTX (missing ExecContext method)

(sqlc is current, afaics)

@Davincible
Copy link

Davincible commented Jan 10, 2023

@alicebob You need to manually build sqlc from main branch and run with experimental flag

$ go install github.com/kyleconroy/sqlc@main
$ $GOPATH/bin/sqlc --experimental generate

@alicebob
Copy link

alicebob commented Jan 10, 2023

@Davincible thanks! I was on @latest, not @main. That fixed it (with the -experimental). Now on with the rest of the pgx4 -> pgx5 changes I need to do...

@BertBR
Copy link

BertBR commented Jan 11, 2023

@Davincible
Just a little tip...

I tried run as you said but i've got this following error:
package github.com/kyleconroy/sqlc is not a main package

Then, i ran as get started documentation saids:
go install github.com/kyleconroy/sqlc/cmd/sqlc@main

And it works!

@Davincible
Copy link

@Davincible
Just a little tip...

I tried run as you said but i've got this following error:
package github.com/kyleconroy/sqlc is not a main package

Then, i ran as get started documentation saids:
go install github.com/kyleconroy/sqlc/cmd/sqlc@main

And it works!

Oh right yes, you're right. Forgot to add the last part

@go-aegian
Copy link

Any estimate date for the docker image be updated containing support for pgx/v5?

@junyan-rippling
Copy link

after yesterday's release, have these issues been all addressed?

@gnuletik
Copy link

I gave a try on pgx/v5 since sqlc@1.17.0 release but I was not able to make array parameters work as @dht-hedaihua mentioned.

From this query:

-- GetAllByIDs :many
SELECT * FROM something WHERE id = ANY($1::uuid[];)

sqlc now generates the following function (diff pgx/v4 vs pgx/v5):

-func (q *Queries) GetAllByIDs(ctx context.Context, dollar_1 []uuid.UUID) ([]*Item, error)
+func (q *Queries) GetAllByIDs(ctx context.Context, dollar_1 pgtype.Array[uuid.UUID]) ([]*Item, error)

However, pgtype.Array needs to be built with Dims and Valid fields.

I think that sqlc should generate this function with a pgtype.FlatArray parameter instead.

@colli173
Copy link
Contributor

colli173 commented Feb 14, 2023

@kyleconroy @mcdoker18 What is the benefit of using the pgtype.FlatArray or pgtype.Array with pgxv5 instead of a standard go slice in models/queries? Here #1651 I was going to represent multidimensional arrays with a standard multi-dim go slice. I was working on implementing the switch to FlatArray/Array today in that same branch, but found the pgtype.Array to be a bit difficult to work with in some code I was testing. And more generally, is it preferred to return a basic go type or a sql_package specific type (e.g. pgtype.Text or pgtype.Array) from query functions and in models by default?

@kyleconroy
Copy link
Collaborator Author

@colli173 sqlc 1.17.2 has been released with greatly improved support for pgx/v5. By default, generated code will use slices []T instead of pgtype.Array[T]. Going to close out this issue in favor of individual issues that for pgx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request proposal
Projects
None yet
Development

No branches or pull requests