jackc / jackc/pgx

Is pgxpool.Tx.{Commit,Rollback} check for tx.c redundant?

Open
#2,410 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.