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

pgx v5 beta 5 - CollectOneRow requires Rows, not Row #1300

Closed
kitsuniru opened this issue Sep 7, 2022 · 3 comments
Closed

pgx v5 beta 5 - CollectOneRow requires Rows, not Row #1300

kitsuniru opened this issue Sep 7, 2022 · 3 comments
Labels

Comments

@kitsuniru
Copy link

Hi, problem described in header

image

go.mod: github.com/jackc/pgx/v5 v5.0.0-beta.5

@kitsuniru kitsuniru added the bug label Sep 7, 2022
@jackc
Copy link
Owner

jackc commented Sep 7, 2022

This is intentional. I had previously considered it, but I think it is better taking Rows.

RowToFunc[T])() takes an interface that Row doesn't satisfy. I don't want to require a parallel set of row mapping functions, one for Query() and one for QueryRow().

Potentially, the Row interface could be expanded to be compatible, but my initial experiments did not end up end up with an improvement in usability. With a normal err = db.QueryRow(...).Scan(...) the Row is never saved to a named variable and the chaining saves a line of code. If CollectOneRow() was used directly with QueryRow() we would end up with something like err = CollectOneRow(db.QueryRow(...), fn). The order of execution is now reversed from the order it is read in the code. Collect happens before query. This strikes me as inelegant. In addition, in real usage it would tend to be too long for a single line. Leading to something like:

row = db.QueryRow(...)
err = pgx.CollectOneRow(row, ...)

But at that point there is no conciseness advantage to using QueryRow() instead of Query().

Since QueryRow() did not appear to provide any advantage over Query() when used with CollectOneRow() it seemed more elegant to me to always use Query().

Another way of viewing it is QueryRow() is basically unnecessary. If I was designing pgx from scratch and wasn't concerned with compatibility with previous versions of pgx or database/sql, I would probably omit QueryRow().

Lastly, a Rows can come from other places than Query(). For example, RowsFromResultReader() can be used to integrate pgtype with pgconn without using Query() or QueryRow(). Building CollectOneRow() on Rows means these other places don't need two versions depending on whether only 1 row is expected.

@kitsuniru
Copy link
Author

kitsuniru commented Sep 7, 2022

Understood your point, thank you!

@ThinhNguyenHoang1
Copy link

ThinhNguyenHoang1 commented Dec 13, 2024

Came across searching the same solution as OP.

I believe this should be documented as the function names doesn't help clarify (in fact they contribute to the belief that this is doable) that we can't query and map a single row like with rows.

Tks for the greate lib though!
@jackc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants