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

release-20.1: pgwire: de-strictify extended protocol handling #51249

Merged

Conversation

jordanlewis
Copy link
Member

Backport 1/1 commits from #51194.

/cc @cockroachdb/release


Fixes #33693.
Fixes #41511.

Previously, the protocol handler didn't permit simple queries during the
extended protocol mode. I think this was done because the spec vaguely
says that extended protocol commands must be ended with a SYNC command:
https://www.postgresql.org/docs/9.3/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY

However, an examination of the PostgreSQL source code shows that the
extended protocol mode bit is only used for error recovery. If an error
is encountered during extended protocol mode in Postgres, all commands
are skipped until the next Sync.

CockroachDB never implemented that behavior - instead, it used the
extended protocol mode bit only to reject simple queries if they came
before the Sync.

This commit removes the extended protocol mode bit, as the use case we
used it for was incorrect. It's unclear to me whether we need to re-add
the bit for dealing with error cases, but we haven't needed it yet.
Adding that might be follow-on work, and won't come in this PR.

Release note (bug fix): prevent spurious "SimpleQuery not allowed while
in extended protocol mode" errors.

Previously, the protocol handler didn't permit simple queries during the
extended protocol mode. I think this was done because the spec vaguely
says that extended protocol commands must be ended with a SYNC command:
https://www.postgresql.org/docs/9.3/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY

However, an examination of the PostgreSQL source code shows that the
extended protocol mode bit is only used for error recovery. If an error
is encountered during extended protocol mode in Postgres, all commands
are skipped until the next Sync.

CockroachDB never implemented that behavior - instead, it used the
extended protocol mode bit only to reject simple queries if they came
before the Sync.

This commit removes the extended protocol mode bit, as the use case we
used it for was incorrect. It's unclear to me whether we need to re-add
the bit for dealing with error cases, but we haven't needed it yet.
Adding that might be follow-on work, and won't come in this PR.

Release note (bug fix): prevent spurious "SimpleQuery not allowed while
in extended protocol mode" errors.
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@jordanlewis jordanlewis merged commit df7992e into cockroachdb:release-20.1 Jul 10, 2020
@jordanlewis jordanlewis deleted the backport20.1-51194 branch July 10, 2020 16:04
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.

None yet

3 participants