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

Fix doc replicated words #1070

Merged
merged 2 commits into from
Aug 23, 2021
Merged

Fix doc replicated words #1070

merged 2 commits into from
Aug 23, 2021

Conversation

smiletrl
Copy link

@smiletrl smiletrl commented Aug 19, 2021

@smiletrl smiletrl changed the title add missing pgx pool release for QueryRow Add missing pgx pool release for QueryRow Aug 19, 2021
@jackc
Copy link
Owner

jackc commented Aug 19, 2021

The connection can't be released until the row is scanned and the release happens when it is scanned. See #1050 for more discussion of the same issue.

@smiletrl
Copy link
Author

smiletrl commented Aug 20, 2021

ah, I see. Could you help describe the race condition if release happens before scan? or how to reproduce the race issue.

@smiletrl
Copy link
Author

smiletrl commented Aug 20, 2021

Right now, QueryRow must be chained by Scan(), otherwise it will eat all available connections. In this case, I feel QueryRow should be removed, but replaced with something like QueryRowScan, which will always release the connection.

Is this struct Pool implementing some interface, which must include QueryRow?

@jackc
Copy link
Owner

jackc commented Aug 21, 2021

Why would QueryRow be called without also calling Scan?

@smiletrl
Copy link
Author

ah, personally I don't have a specific case without calling Scan. Just feel it doesn't look right that if one function must be chained by another one, otherwise it could break the application, like bug reported at #1050.

@smiletrl smiletrl changed the title Add missing pgx pool release for QueryRow Fix doc replicated words Aug 23, 2021
@smiletrl
Copy link
Author

This release is removed now^ This pr only fixes that doc.

@jackc jackc merged commit 5320ad8 into jackc:master Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants