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

cli/sql: prompting fixes #105137

Merged
merged 1 commit into from Jun 21, 2023
Merged

cli/sql: prompting fixes #105137

merged 1 commit into from Jun 21, 2023

Conversation

knz
Copy link
Contributor

@knz knz commented Jun 19, 2023

Fixes #102153.
Fixes #105135.
Epic: CRDB-28893

Release note (backwards-incompatible change): When customizing the SQL interactive prompt, the special sequence %M now expands to the full host name instead of the combination of host name and port number. To include the port number explicitly, use %>. The special sequence %m now expands to the host name up to the first period.

Release note (cli change): When customizing the SQL interactive prompt, %M and %m now behave more like psql when connecting over a unix datagram socket.

@knz knz requested a review from rafiss June 19, 2023 11:26
@knz knz requested review from a team as code owners June 19, 2023 11:26
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Release note (backwards-incompatible change): When customizing the SQL
interactive prompt, the special sequence `%M` now expands to the full
host name instead of the combination of host name and port number. To
include the port number explicitly, use `%>`. The special sequence
`%m` now expands to the host name up to the first period.

Release note (cli change): When customizing the SQL interactive
prompt, `%M` and `%m` now behave more like `psql` when connecting
over a unix datagram socket.
Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @knz)


pkg/cli/interactive_tests/test_txn_prompt.tcl line 31 at r1 (raw file):

send "\\set prompt1 abc%mdef\r"
eexpect abc127def

all the logic lgtm, but just a sanity check about what PG is doing: does PG always truncate up to the first period like this? i was wondering if it handled hostnames different from IP addresses.

@knz
Copy link
Contributor Author

knz commented Jun 21, 2023

all the logic lgtm, but just a sanity check about what PG is doing: does PG always truncate up to the first period like this?

It's literally in the doc:

The host name of the database server, truncated at the first dot, or [local] if the connection is over a Unix domain socket.

Also in the pg source code, src/bin/psql/prompt.c:

                        /* INET socket */
                        if (host && host[0] && !is_unixsock_path(host))
                        {
                            strlcpy(buf, host, sizeof(buf));
                            if (*p == 'm')
                                buf[strcspn(buf, ".")] = '\0';
                        }

Looks like quite specifically "first dot" to me.

@knz
Copy link
Contributor Author

knz commented Jun 21, 2023

bors r=rafiss

@craig
Copy link
Contributor

craig bot commented Jun 21, 2023

Build succeeded:

@craig craig bot merged commit 39fbe73 into cockroachdb:master Jun 21, 2023
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants