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: don't refresh prompt on line continuations #61207

Merged
merged 1 commit into from
Mar 5, 2021

Conversation

jordanlewis
Copy link
Member

Previously, the CLI would recalculate its prompt (including any required
network roundtrips to determine transaction status and current database)
on every newline, even if the newline came in the middle of a SQL
statement. This was wasteful, of course, but it's also very confusing
for users when entering a large multi-line statement via paste: the
prompt refreshes defer until enter is pressed for the first time, and
there is a lot of unexplained latency during the statement that doesn't
reflect the actual SQL being executed. It also doesn't match the
reported execution time, leading to mass confusion and sadness.

Now, we don't bother refreshing the prompt during line continuations.

Closes #61095

Release note (cli change): optimize handling of multi-line SQL strings
to avoid unwanted extra server roundtrips.

Release justification: bug fixes and low-risk updates to new functionality

Previously, the CLI would recalculate its prompt (including any required
network roundtrips to determine transaction status and current database)
on every newline, even if the newline came in the middle of a SQL
statement. This was wasteful, of course, but it's also very confusing
for users when entering a large multi-line statement via paste: the
prompt refreshes defer until enter is pressed for the first time, and
there is a lot of unexplained latency during the statement that doesn't
reflect the actual SQL being executed. It also doesn't match the
reported execution time, leading to mass confusion and sadness.

Now, we don't bother refreshing the prompt during line continuations.

Release note (cli change): optimize handling of multi-line SQL strings
to avoid unwanted extra server roundtrips.

Release justification: bug fixes and low-risk updates to new functionality
@jordanlewis jordanlewis requested review from knz and a team February 26, 2021 21:04
@jordanlewis jordanlewis requested a review from a team as a code owner February 26, 2021 21:04
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@jordanlewis
Copy link
Member Author

bors r+

@craig craig bot merged commit 90d6ba4 into cockroachdb:master Mar 5, 2021
@craig
Copy link
Contributor

craig bot commented Mar 5, 2021

Build succeeded:

@jordanlewis jordanlewis deleted the fix-cli-prompt-refresh-issue branch March 26, 2021 19:02
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.

cli: incorrect query latencies reported by CLI
3 participants