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

pgrepl: parse replication statements + handle IDENTIFY_SYSTEM #106131

Merged
merged 4 commits into from
Aug 8, 2023

Conversation

otan
Copy link
Contributor

@otan otan commented Jul 4, 2023

Informs: #105130

pgwire: ban extended protocol with REPLICATION protocol

In line with PostgreSQL, we prohibit the use of the extended protocol
when the replication protocol is in effect.

Release note: None

pgrepl: implement IDENTIFY_SYSTEM command

This commit implements IDENTIFY_SYSTEM in the replication protocol,
which is used to retrieve metadata about replication state.

Most of this commit involves changing assumptions throughout the
codebase there is 1 parser - there is 2 if replication mode is enabled.
We've also had to change the parser to return statements.Statement
instead of pgrepltree.ReplicationStatement for compatibility with
parser.Parse....

I've left the LSN fields as placeholders for now as we finalise on
what we'll eventually call it.

Release note: None

sql: no-op SHOW SYNTAX in replication mode

Currently on the CLI, the command must parse on the server for it to
run. For replication related commands, we have not implemented SHOW SYNTAX yet. This is a bit of effort and seems outside the scope of what
we want for now.

For now, handle replication commands by no-oping them.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@otan otan force-pushed the extended_proto branch from 88fed94 to df9be31 Compare July 5, 2023 00:03
@otan otan force-pushed the extended_proto branch 2 times, most recently from f1d1279 to fafa157 Compare July 5, 2023 03:54
@otan otan force-pushed the extended_proto branch from fafa157 to d7d9ffc Compare July 18, 2023 05:02
@otan otan requested a review from rafiss July 18, 2023 05:03
@otan otan marked this pull request as ready for review July 18, 2023 05:03
@otan otan requested review from a team as code owners July 18, 2023 05:03
@otan otan requested a review from cucaroach July 18, 2023 05:03
Copy link
Contributor

@cucaroach cucaroach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 7 of 7 files at r1, 21 of 21 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rafiss)

@otan otan force-pushed the extended_proto branch 3 times, most recently from 7305eed to f7fe19a Compare August 7, 2023 13:46
otan added 4 commits August 8, 2023 09:45
In line with PostgreSQL, we prohibit the use of the extended protocol
when the replication protocol is in effect.

Release note: None
This commit implements IDENTIFY_SYSTEM in the replication protocol,
which is used to retrieve metadata about replication state.

Most of this commit involves changing assumptions throughout the
codebase there is 1 parser - there is 2 if replication mode is enabled.
We've also had to change the parser to return `statements.Statement`
instead of `pgrepltree.ReplicationStatement` for compatibility with
`parser.Parse...`.

I've left the `LSN` fields as placeholders for now as we finalise on
what we'll eventually call it.

Release note: None
Currently on the CLI, the command must parse on the server for it to
run. For replication related commands, we have not implemented `SHOW
SYNTAX` yet. This is a bit of effort and seems outside the scope of what
we want for now.

For now, handle replication commands by no-oping them.

Release note: None
@otan
Copy link
Contributor Author

otan commented Aug 8, 2023

bors r=cucaroach

@craig
Copy link
Contributor

craig bot commented Aug 8, 2023

Build succeeded:

@craig craig bot merged commit 8e614fc into cockroachdb:master Aug 8, 2023
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.

3 participants