-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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: add some additional client-side commands #54796
Conversation
There was a problem hiding this 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, 2 of 2 files at r2, 2 of 2 files at r3, 2 of 2 files at r4, 7 of 7 files at r5.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @arulajmani and @knz)
pkg/cli/sql.go, line 1050 at r5 (raw file):
return c.runSyscmd(c.lastInputLine, loopState, errState) case `\i`:
looks like \ir
may be a cool follow up.
pkg/cli/sql.go, line 1124 at r5 (raw file):
const maxRecursionLevels = 10 func (c *cliState) runRecursive(
nit: had a bit of trouble understanding what this function did. maybe importCLICommandFile
? the recursion concept is not evident here because this function doesn't actually do any recursion.
pkg/cli/sql.go, line 1132 at r5 (raw file):
if c.levels >= maxRecursionLevels { c.exitErr = errors.New(`\i: too many recursion levels`)
nit: useful to print max recursion level here. maybe even be more descriptive, e.g. recursed into too many files running \i (maximum %d)
pkg/cli/sql.go, line 1151 at r5 (raw file):
defer func() { if err := f.Close(); err != nil { fmt.Fprintf(stderr, "closing %s: %v\n", filename, err)
nit: does some sort of ERROR
thing show up in front of this? it may look confusing otherise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @arulajmani and @otan)
pkg/cli/sql.go, line 1050 at r5 (raw file):
Previously, otan (Oliver Tan) wrote…
looks like
\ir
may be a cool follow up.
Done.
pkg/cli/sql.go, line 1124 at r5 (raw file):
Previously, otan (Oliver Tan) wrote…
nit: had a bit of trouble understanding what this function did. maybe
importCLICommandFile
? the recursion concept is not evident here because this function doesn't actually do any recursion.
Renamed as suggested.
pkg/cli/sql.go, line 1132 at r5 (raw file):
Previously, otan (Oliver Tan) wrote…
nit: useful to print max recursion level here. maybe even be more descriptive, e.g.
recursed into too many files running \i (maximum %d)
Done.
pkg/cli/sql.go, line 1151 at r5 (raw file):
Previously, otan (Oliver Tan) wrote…
nit: does some sort of
ERROR
thing show up in front of this? it may look confusing otherise
Done.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 10 files at r6, 1 of 2 files at r7, 1 of 2 files at r8, 1 of 2 files at r9, 8 of 8 files at r10.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @arulajmani and @otan)
Release note (cli change): The help text displayed upon `\?` in `cockroach sql` / `cockroach demo` now groups the recognized client-side commands into sections for easier reading.
Release note (cli change): The client-side command `\show` for the SQL shell is deprecated in favor of the new command `\p`. This prints the contents of the query buffer entered so far. Release note (cli change): The new client-side command `\r` for the SQL shell erases the contents of the query buffer entered so far. This provides a convenient way to reset the input e.g. when the user gets themselves confused with string delimiters.
Ahead of implementing recursive evaluation, we need to ensure that all variables customizable via `\set` / `\unset` become independent of `cliState`. This patch does it. Release note: None
Release note (cli change): The SQL shell (`cockroach sql`, `cockroach demo`) now supports the client-side command `\echo`, like `psql`. This can be used e.g. to generate informational output when executing SQL scripts non-interactively.
Release note (cli change): The SQL shell (`cockroach sql`, `cockroach demo`) now support the `\i` and `\ir` client-side command which reads SQL file and evaluates its content in-place. `\ir` differs from `\i` in that the file name is resolved relative to the location of the script containing the `\ir` command. This makes `\ir` likely more desirable in the general case. Occurences of `\q` inside a file included via `\i`/`\ir stop evaluation of the file and resume evaluation of the file that included it. This feature is compatible with the identically named `psql` commands. It is meant to help compose complex initialization scripts from a library of standard components. For example, one could be defining each table and its initial contents in separate SQL files, and then use different super-files to include different tables depending on the desired final schema.
TFYR! bors r=otan |
Build succeeded: |
Fixes #25178
Supports use cases promoted by #54771
Adds
\p
,\r
\echo
and especially\i
/\ir
(recursive include).