georgysavva / scany

Library for scanning data from a database into Go structs and more
MIT License
1.24k stars 67 forks source link

Are there any plans to support pgx v5? #88

Closed jnst closed 1 year ago

jnst commented 1 year ago

In a month or so, pgx v5 will be officially released.

Here are the commits I supported v5 for reference.

pgx v5 requires go 1.18 or higher. do you plan to support this in scany?

georgysavva commented 1 year ago

Hi. Thank you for bringing this up. Supporting pgx v5 should be a good improvement; I see no obstacles doing it. If you already did so in your version of the repo, may I ask you to shoot a PR with the change?

jnst commented 1 year ago

@georgysavva To support pgx v5, it means it has to be go v1.18 to work. Does scany think it is ok to discontinue lower versions of Go?

Edwing123 commented 1 year ago

Hello, @georgysavva, In Pgx v5, the pgx.Rows interface has a new method called Conn and it returns *Conn, right now Scany is using Pgx v4, so I think it would be cool to update to version 5.

error-message pgx-v5-rows-interface

Thanks for the library :+1:

georgysavva commented 1 year ago

Yeah, supporting pgx v5 is indeed a desirable improvement. However, I need to think about this:

To support pgx v5, it means it has to be go v1.18 to work. Does scany think it is ok to discontinue lower versions of Go?

How to do the transaction gracefully. To ensure backward compatibility of scany API, it would need to be scany v2 as well, I assume?

cc: @jnst, @Edwing123

ItalyPaleAle commented 1 year ago

@georgysavva I depend on this (awesome) project too and I would really love to see pgx v5 support.

With Go 1.19 being the latest version now, personally I am not too concerned with dropping support for Go < 1.18.

However, because the APIs for pgx have changed a lot and are incompatible, perhaps releasing scany v2 would not be a bad idea regardless. Then, scany "v1" would continue to work with pgx v4.

Edwing123 commented 1 year ago

Yes, releasing Scany v2 would a good thing to let v1 be compatible with older Go versions.

georgysavva commented 1 year ago

Let's go with the scany v2 release then.

I just check the pgx repo and it says that pgx v5 is in beta and will be released in September. Is it okay for you if we do the scany v2 release after pgx v5 is released and stable?

Edwing123 commented 1 year ago

Let's go with the scany v2 release then.

I just check the pgx repo and it says that pgx v5 is in beta and will be released in September. Is it okay for you if we do the scany v2 release after pgx v5 is released and stable?

I wonder if releasing scany v2 beta would be a good starting point. When trying scany with pgx v5, it was only a few compatibility errors because of new added fields or renamed methods, but yeah, more breaking changes could be added to pgx v5, and that could get tiresome for you to update scany every time more changes are added while it's in beta.

Edwing123 commented 1 year ago

Well, September is almost here, in my case, I wouldn't mind waiting for pgx v5 to become stable, and since there's not a lot of people having problems with scany (just a few have commented in this issue)

georgysavva commented 1 year ago

Then let's wait for the stable pgx v5 release. Feel free to ping me when it's out there, in case I've missed it.

kitsuniru commented 1 year ago

@georgysavva pgx v5 now have a built-in method pgx.RowToStructByPos[T], which deserializes the resulting Rows into a struct from T generic.

Examples:

Many rows:

rows, err := conn.Query(ctx, sql)
if err != nil {
...
}
// somearray will be []model.Person
somearray, err := pgx.CollectRows[model.Person](rows, pgx.RowToStructByPos[model.Person])
if err != nil {
...
}

Single row:


row, err := conn.Query(ctx, sql)
if err != nil {
...
}
// someperson will be model.Person
someperson, err := pgx.CollectOneRow[model.Person](row, pgx.RowToStructByPos[model.Person])
if err != nil {
...
}

Warning: fields in struct and row must be in the same ordering

Keep it in mind that there's no QueryRow() for single value, because maintainer (@jackc) recommend to use Query() rather than QueryRow()

https://github.com/jackc/pgx/issues/1300#issuecomment-1239314987

vadimi commented 1 year ago

seems like pgx v5 got released: https://github.com/jackc/pgx/releases/tag/v5.0.0

lzap commented 1 year ago

Guys, it can probably take a while so I would appreciate creating a v2-dev branch so we could start using the first version and report errors. Pgx v5 is a big milestone for those who want to incorporate OpenTelemetry :-)

georgysavva commented 1 year ago

Here everyone. I created a pre-release for scany/v2: https://github.com/georgysavva/scany/releases/tag/v2.0.0-alpha.2. You can install it the following way: go get https://github.com/georgysavva/scany/v2@v2.0.0-alpha.2.

Please test it out a little bit, and if everything is fine, I will make the stable release.

vadimi commented 1 year ago

it looks like github.com/jackc/pgtype should be replaced with github.com/jackc/pgx/v5/pgtype according to https://github.com/jackc/pgx/blob/f803c790d0999e9028cbffefad9df02e4aff913a/CHANGELOG.md#merged-packages, so I just submitted https://github.com/georgysavva/scany/pull/97

georgysavva commented 1 year ago

Here is the new pre-release of scany/v2 after @vadimi's changes: https://github.com/georgysavva/scany/releases/tag/v2.0.0-alpha.3

go get https://github.com/georgysavva/scany/v2@v2.0.0-alpha.3
georgysavva commented 1 year ago

Hi guys! Since no one has reported any issues with the scany/v2 pre-release, I went ahead and published the first stable scany/v2 release: https://github.com/georgysavva/scany/releases/tag/v2.0.0. You can update via:

go get https://github.com/georgysavva/scany/v2

Thank you to everyone who participated in the transition, and especially to @vadimi, who took the initiative from the beginning till the end!

Feel free to reopen this issue or create a new one if something is wrong with scany/v2.

lzap commented 1 year ago

Works fine for our use case, but we barely scratch the surface of the features. Just few tables and the most complex code is actually storing a single JSONB field :-)

Thanks!