Skip to content

Clearer error starting a query when already processing a query#1272

Merged
arp242 merged 1 commit intomasterfrom
sync
Mar 10, 2026
Merged

Clearer error starting a query when already processing a query#1272
arp242 merged 1 commit intomasterfrom
sync

Conversation

@arp242
Copy link
Copy Markdown
Collaborator

@arp242 arp242 commented Mar 10, 2026

pq's implementation of the PostgreSQL query protocol is fairly straight forward and doesn't support running more than one query at a time.

This means that something like this:

tx, _ := db.Begin()

rows, _ := tx.Query(..)
for rows.Next() {
	tx.Query(..) // Sends query while we're reading the first.
}

Will fail, as it sends a query while we're still reading the first query.

We could use the extended query protocol, which does allow running more than one query without waiting for the previous one to complete. It's an option, I guess, but reading the docs it also seems rather more complex – I kind of like how straight-forward pq's implementation is right now and anyone can step through it without much understanding of PostgreSQL.

The "classic" solution for this is to just immediately read all rows in memory, which is what many PostgreSQL drivers do. I guess we could add an option for that (and there's a PR for it at #117). But idk, seems easy enough to do manually? And we can't do it by default, since people have written code against pq with the assumption that it won't read all data in memory: so it will still catch people not aware of this.

So anyway, just change the error message to be clearer. It hasn't come up that often in the last ~13 years so it's probably "good enough", and it's the simplest way to improve this.

There was already inCopy to prevent other queries being run while COPY was in progress, so expand and re-use that. We reset it when we get ReadyForQuery from the server. Not sure if atomic.Bool is needed or just bool will be fine? I couldn't trigger a race in my testing, but err is protected with syncErr, and it doesn't hurt to be safe.

Note we can still do any of the above (extended query protocol, read all in memory) later: this is not preventing that.

Fixes #81
Fixes #117
Probably fixes #855
Probably fixes #894

pq's implementation of the PostgreSQL query protocol is fairly straight
forward and doesn't support running more than one query at a time.

This means that something like this:

	tx, _ := db.Begin()

	rows, _ := tx.Query(..)
	for rows.Next() {
		tx.Query(..) // Sends query while we're reading the first.
	}

Will fail, as it sends a query while we're still reading the first
query.

We could use the [extended query protocol], which does allow running
more than one query without waiting for the previous one to complete.
It's an option, I guess, but reading the docs it also seems rather more
complex – I kind of like how straight-forward pq's implementation is
right now and anyone can step through it without much understanding of
PostgreSQL.

The "classic" solution for this is to just immediately read all rows in
memory, which is what many PostgreSQL drivers do. I guess we could add
an option for that (and there's a PR for it at #117). But idk, seems
easy enough to do manually? And we can't do it by default, since people
have written code against pq with the assumption that it won't read all
data in memory: so it will still catch people not aware of this.

So anyway, just change the error message to be clearer. It hasn't come
up that often in the last ~13 years so it's probably "good enough", and
it's the simplest way to improve this.

There was already inCopy to prevent other queries being run while COPY
was in progress, so expand and re-use that. We reset it when we get
ReadyForQuery from the server. Not sure if atomic.Bool is needed or just
bool will be fine? I couldn't trigger a race in my testing, but err is
protected with syncErr, and it doesn't hurt to be safe.

Note we can still do any of the above (extended query protocol, read all
in memory) later: this is not preventing that.

[extended query protocol]: https://www.postgresql.org/docs/current/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY

Fixes #81
Fixes #117
Probably fixes #855
Probably fixes #894
@arp242 arp242 merged commit c66ad00 into master Mar 10, 2026
13 checks passed
@arp242 arp242 deleted the sync branch March 10, 2026 01:27
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.

Response to COPY command is interpreted as error unknown response during CopyIn: 'S' Can't run two queries in a transaction?

1 participant