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

Add PostgreSQL REPL implementation #49598

Merged
merged 12 commits into from
Dec 13, 2024
Merged

Add PostgreSQL REPL implementation #49598

merged 12 commits into from
Dec 13, 2024

Conversation

gabrielcorado
Copy link
Contributor

Part of #44956 (RFD 0181)

This PR introduces the PostgreSQL REPL implementation following the RFD definition, which will be used as a client to access PostgreSQL instances through WebUI. It works like a terminal emulator (implemented by golang.org/x/term library) and will be used with the WebUI terminal (like WebUI SSH sessions).

Note

This PR only includes the REPL implementation. The web handlers and WebUI changes will be done in different PRs.

For this first release of the REPL, we're not including informational commands (such as \d or \dt). These commands will require better testing across different PostgreSQL versions and will be introduced in later versions.

@gabrielcorado gabrielcorado added the no-changelog Indicates that a PR does not require a changelog entry label Nov 30, 2024
lib/client/db/postgres/repl/repl.go Outdated Show resolved Hide resolved
lib/client/db/postgres/repl/repl.go Outdated Show resolved Hide resolved
lib/client/db/postgres/repl/commands.go Show resolved Hide resolved
@flyinghermit flyinghermit removed their request for review December 2, 2024 14:37
Copy link
Contributor

@greedy52 greedy52 left a comment

Choose a reason for hiding this comment

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

First pass. Overall logic looks good to me.

A few things i wonder if we could do (not immediately but maybe after MVP):

  1. common REPL so it can be used for other protocols like MySQL
  2. test postgres repl (or even do this through the webapi websocket) in E2E test (or testcontainers) against real RDS/Postgres

lib/client/db/postgres/repl/repl.go Outdated Show resolved Hide resolved
lib/client/db/postgres/repl/commands.go Outdated Show resolved Hide resolved
lib/client/db/postgres/repl/commands.go Outdated Show resolved Hide resolved
lib/client/db/postgres/repl/commands_test.go Show resolved Hide resolved
lib/client/db/postgres/repl/commands_test.go Outdated Show resolved Hide resolved
switch {
case strings.HasPrefix(line, commandPrefix) && !readingMultiline:
var exit bool
reply, exit = r.processCommand(line)
Copy link
Contributor

Choose a reason for hiding this comment

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

write non-empty reply before exit?

lib/client/db/postgres/repl/commands.go Outdated Show resolved Hide resolved
lib/client/db/postgres/repl/repl_test.go Outdated Show resolved Hide resolved
lib/client/db/postgres/repl/repl_test.go Outdated Show resolved Hide resolved
lib/client/db/postgres/repl/repl_test.go Outdated Show resolved Hide resolved
@gabrielcorado
Copy link
Contributor Author

@greedy52

common REPL so it can be used for other protocols like MySQL

We could do this in the future (when we introduce another database). However, it is a bit difficult to identify which part we can reuse right now.

test postgres repl (or even do this through the webapi websocket) in E2E test (or testcontainers) against real RDS/Postgres

I'll add the integration and E2E tests after we have both parts (web handler and REPL) ready/merged. So we can thoroughly test their usage against the databases.

lib/client/db/postgres/repl/repl.go Outdated Show resolved Hide resolved
lib/client/db/postgres/repl/commands.go Outdated Show resolved Hide resolved
lib/client/db/postgres/repl/repl_test.go Show resolved Hide resolved
lib/client/db/postgres/repl/commands.go Outdated Show resolved Hide resolved
lib/client/db/postgres/repl/repl.go Outdated Show resolved Hide resolved
lib/client/db/postgres/repl/repl.go Show resolved Hide resolved
lib/client/db/postgres/repl/repl.go Outdated Show resolved Hide resolved
@gabrielcorado gabrielcorado requested a review from zmb3 December 10, 2024 15:06
@gabrielcorado gabrielcorado added this pull request to the merge queue Dec 13, 2024
Merged via the queue into master with commit 89ea69c Dec 13, 2024
42 checks passed
@gabrielcorado gabrielcorado deleted the gabrielcorado/pg-repl branch December 13, 2024 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Indicates that a PR does not require a changelog entry size/lg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants