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

Document cockroach-sql binary #14671

Merged
merged 12 commits into from
Aug 23, 2022
Merged

Document cockroach-sql binary #14671

merged 12 commits into from
Aug 23, 2022

Conversation

ianjevans
Copy link
Contributor

Fixes DOC-3634.

@ianjevans ianjevans marked this pull request as draft July 29, 2022 20:16
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@netlify
Copy link

netlify bot commented Jul 29, 2022

Netlify Preview

Name Link
🔨 Latest commit fc1a646
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/63040814f165cb000bd6383f
😎 Deploy Preview https://deploy-preview-14671--cockroachdb-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@ianjevans ianjevans force-pushed the DOC-3634-cockroach-sql branch from 20ec70b to 59cbeae Compare August 18, 2022 16:55
@ianjevans ianjevans marked this pull request as ready for review August 18, 2022 23:35
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 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @gemma-shay, @ianjevans, @nickvigilante, and @rail)


v22.1/cockroach-sql-binary.md line 74 at r1 (raw file):

~~~ shell
$ cockroach-sql --execute="<sql statement>;<sql statement>" --execute="<sql-statement>" <flags>

I personally prefer to teach the short argument form (-e / -f), your call.


v22.1/cockroach-sql-binary.md line 86 at r1 (raw file):

~~~ sql
> \q

let's just document this one.


v22.1/cockroach-sql-binary.md line 126 at r1 (raw file):

`--echo-sql` | Reveal the SQL statements sent implicitly by the command-line utility. For a demonstration, see the [example](#reveal-the-sql-statements-sent-implicitly-by-the-command-line-utility) below.<br><br>This can also be enabled within the interactive SQL shell via the `\set echo` [shell command](#commands).
<a name="sql-flag-execute"></a> `--execute`<br><br>`-e` | Execute SQL statements directly from the command line, without opening a shell. This flag can be set multiple times, and each instance can contain one or more statements separated by semi-colons. If an error occurs in any statement, the command exits with a non-zero status code and further statements are not executed. The results of each statement are printed to the standard output (see `--format` for formatting options).<br><br>For a demonstration of this and other ways to execute SQL from the command line, see the [example](#execute-sql-statements-from-the-command-line) below.
`--file <filename>`<br><br>`-f <filename>`<br><br>`< <filename>` |  Read SQL statements from `<filename>`.

The redirection form with < is a completely different feature! And is not part of the CLI interface. Please don't include it here.


v22.1/cockroach-sql-binary.md line 136 at r1 (raw file):

### Client connection

{% include {{ page.version.version }}/sql/connection-parameters.md %}

It would be good to re-order this table to promote --url first as the preferred approach, then the discrete parameters as an alternative to the URL.


v22.1/cockroach-sql-binary.md line 148 at r1 (raw file):

`cockroach-sql` exhibits different behaviors depending on whether or not the session is interactive and/or whether or not the session outputs on a terminal.

- A session is **interactive** when `cockroach-sql` is invoked without the `--execute` flag and input is not redirected from a file. In such cases:

"... without -e or -f, and the input is a terminal"


v22.1/cockroach-sql-binary.md line 152 at r1 (raw file):

    - The [`check_syntax` option](#sql-option-check-syntax) defaults to `true` if supported by the CockroachDB server (this is checked when the shell starts up).
    - **Ctrl+C** at the prompt will only terminate the shell if no other input was entered on the same line already.
    - The shell will attempt to set the `safe_updates` [session variable](set-vars.html) to `true` on the server.

Missing point:

"""

  • The shell continues to read input after the last command entered.
    """

v22.1/cockroach-sql-binary.md line 158 at r1 (raw file):

When a session is both interactive and outputs on a terminal, `cockroach-sql` also activates the interactive prompt with a line editor that can be used to modify the current line of input. Also, command history becomes active.

This section is missing an explanation on non-interactive behavior, which is different.


v22.1/cockroach-sql-binary.md line 159 at r1 (raw file):

When a session is both interactive and outputs on a terminal, `cockroach-sql` also activates the interactive prompt with a line editor that can be used to modify the current line of input. Also, command history becomes active.

## SQL shell

Somewhere in this page, mention should be made that the output is part of a stable interface, except informational output lines that start with # (i.e. users should not make programmatic use of it).


v22.1/cockroach-sql-binary.md line 226 at r1 (raw file):

- If you are developing automation that uses the CockroachDB SQL shell, it is more reliable to check for `SQLSTATE` values than for error message strings, which are likely to change.

## Examples

I wonder - could we perhaps share this content with the other page? This way they won't drift from each other as we add more features.


v22.1/cockroach-sql-binary.md line 244 at r1 (raw file):

{% include_cached copy-clipboard.html %}
~~~ shell
# Using the --url flag:

Maybe worth demonstrating the URL first.

Copy link
Member

@rail rail 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 @gemma-shay, @ianjevans, and @nickvigilante)


v22.1/cockroach-sql-binary.md line 128 at r1 (raw file):

`--file <filename>`<br><br>`-f <filename>`<br><br>`< <filename>` |  Read SQL statements from `<filename>`.
<a name="sql-flag-format"></a> `--format` | How to display table rows printed to the standard output. Possible values: `tsv`, `csv`, `table`, `raw`, `records`, `sql`, `html`.<br><br>**Default:** `table` for sessions that [output on a terminal](#session-and-output-types); `tsv` otherwise<br /><br />This flag corresponds to the `display_format` [client-side option](#client-side-options).
`--read-only` | **New in v22.1:** Sets the `default_transaction_read_only` [session variable](show-vars.html#supported-variables) to `on` upon connecting.

A nit. We do not ship cockroach-sql with prior versions, so the note is redundant.

Code quote:

**New in v22.1:**

Copy link
Contributor

@nickvigilante nickvigilante left a comment

Choose a reason for hiding this comment

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

Can you replicate this to v22.2 as well? Otherwise, LGTM!

v22.1/cockroach-sql-binary.md Show resolved Hide resolved
v22.1/cockroach-sql-binary.md Show resolved Hide resolved
@ianjevans
Copy link
Contributor Author

@knz Incorporated all your feedback.

@rail I removed the inline new in, and there's now a notice at the top of the topic.

@nickvigilante I'm so glad you figured the <div> issue out. It was driving me crazy.

For ease of review I will move all these changes to the v22.2 files after I get approval.

Copy link
Member

@rail rail left a comment

Choose a reason for hiding this comment

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

LGTM, :shipit:

Copy link
Contributor

@nickvigilante nickvigilante left a comment

Choose a reason for hiding this comment

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

That was a long one. See my comments and reach out if you have any questions.

_includes/v22.1/sql/sql-examples.md Outdated Show resolved Hide resolved
_includes/v22.1/sql/sql-examples.md Show resolved Hide resolved
_includes/v22.1/sql/sql-examples.md Outdated Show resolved Hide resolved
_includes/v22.1/sql/sql-examples.md Outdated Show resolved Hide resolved
_includes/v22.1/sql/sql-examples.md Outdated Show resolved Hide resolved
_includes/v22.1/sql/sql-examples.md Show resolved Hide resolved
v22.1/cockroach-sql-binary.md Show resolved Hide resolved
v22.1/cockroach-sql-binary.md Outdated Show resolved Hide resolved
v22.1/cockroach-sql-binary.md Show resolved Hide resolved
v22.1/cockroach-sql-binary.md Outdated Show resolved Hide resolved
Ian Evans and others added 2 commits August 19, 2022 17:09
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 3 of 5 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @gemma-shay, @ianjevans, and @nickvigilante)


_includes/v22.1/sql/sql-examples.md line 404 at r3 (raw file):

~~~

### Edit SQL statements in an external editor

Please do not include this section in the doc, as we're going to remove this functionality imminently. (cockroachdb/cockroach#86457)

However, separately from this doc project, I'd like to hear your opinion on this: is the ability to use an external editor desirable? Do you think users will be grateful if we streamline this a little bit?


_includes/v22.1/sql/sql-examples.md line 595 at r3 (raw file):

~~~ shell
$ cockroach-sql --url='postgres://root@?host=/tmp&port=26257'
~~~

missing newline at end of file


v22.1/cockroach-sql.md line 17 at r3 (raw file):

{{site.data.alerts.end}}

The output of `cockroach sql` is part of a stable interface, and can be used programmatically, with the exception of informational output lines that begin with the hash symbol (`#`). Informational output can change from release to release, and should not be used programmatically.

The output of cockroach sql when used non-interactively is part of ...


v22.1/cockroach-sql-binary.md line 20 at r3 (raw file):

To exit the interactive shell, use `\q`, `quit`, `exit`, or `ctrl-d`.

The output of `cockroach-sql` is part of a stable interface, and can be used programmatically, with the exception of informational output lines that begin with the hash symbol (`#`). Informational output can change from release to release, and should not be used programmatically.

The output of cockroach sql when used non-interactively is part of ...

(the structure of the input and output during interactive use is unstable)

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.

:lgtm:

Reviewed 2 of 3 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @gemma-shay, @ianjevans, and @nickvigilante)

@@ -81,7 +83,7 @@ Flag | Description
`--embedded` | Minimizes the SQL shell [welcome text](#welcome-message) to be appropriate for embedding in playground-type environments. Specifically, this flag removes details that users in an embedded environment have no control over (e.g., networking information).
`--echo-sql` | Reveal the SQL statements sent implicitly by the command-line utility. For a demonstration, see the [example](#reveal-the-sql-statements-sent-implicitly-by-the-command-line-utility) below.<br><br>This can also be enabled within the interactive SQL shell via the `\set echo` [shell command](#commands).
<a name="sql-flag-execute"></a> `--execute`<br><br>`-e` | Execute SQL statements directly from the command line, without opening a shell. This flag can be set multiple times, and each instance can contain one or more statements separated by semi-colons. If an error occurs in any statement, the command exits with a non-zero status code and further statements are not executed. The results of each statement are printed to the standard output (see `--format` for formatting options).<br><br>For a demonstration of this and other ways to execute SQL from the command line, see the [example](#execute-sql-statements-from-the-command-line) below.
`--file <filename>`<br><br>`-f <filename>`<br><br>`< <filename>` | Read SQL statements from `<filename>`.
`--file <filename>`<br><br>`-f <filename>` | Read SQL statements from `<filename>`.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should standardize the format for having two items in one cell. In Third Party Tools we have line breaks (https://www.cockroachlabs.com/docs/stable/third-party-database-tools.html#application-frameworks), here we're using whitespace, and lower on the page we're using commas. I'm somewhat partial to the line breaks but open to anything as long as its consistent. Commas do seem to be working well here: https://deploy-preview-14671--cockroachdb-docs.netlify.app/docs/v22.1/cockroach-sql.html#commands.

@ianjevans ianjevans merged commit 045c0c0 into master Aug 23, 2022
@ianjevans ianjevans deleted the DOC-3634-cockroach-sql branch August 23, 2022 22:41
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.

6 participants