Is pgxpool.Tx.{Commit,Rollback} check for tx.c redundant?
- Dominant language
- Go
- Stars
- 14.3k
- Forks
- 1.1k
- Avg merge
- 6d 9h
- Merged PRs (30d)
- 11
Description
`pgxpool.Tx.{Commit,Rollback}` contain the following snippet (https://github.com/jackc/pgx/blob/v5.7.6/pgxpool/tx.go#L25):
```go
if tx.c != nil {
tx.c.Release()
tx.c = nil
}
```
However the documentation for `pgxpool.Conn.Release` says
"However, it is safe to call Release multiple times. Subsequent calls after the first will be ignored."
and the implementation exits quickly if the `Release` was called already (https://github.com/jackc/pgx/blob/v5.7.6/pgxpool/conn.go#L20):
```go
if c.res == nil {
return
}
conn := c.Conn()
res := c.res
c.res = nil
```
So is the checking in `Tx` redundant?
Contributor guide
Research direction
Start by reading pgxpool/tx.go at the Commit and Rollback code referenced in the issue, then compare it with pgxpool/conn.go and the documented Release behavior. Determine whether the checks are redundant and identify the relevant existing tests, if any, that establish the expected repeated-release behavior; done means the issue has a clear conclusion and matching test coverage or a focused cleanup.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, postgresql
- Domain
- database
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100