Merge #54741 #54751

54741: cli: support `-f` to read SQL from a file r=irfansharif a=knz

Fixes #42955

Release note (cli change): `cockroach sql` and `cockroach demo` now
support the command-line parameter `--input-file` (shorthand `-f`) to
read commands from a named file. The behavior is the same as if the
file was redirected on the standard input; in particular, the
processing stops at the first error encountered (which is different
from interactive usage with a prompt).

Note that it is not (yet) possible to combine `-f` with `-e`.

54751: cli/sql: further polish statement statistics r=irfansharif,arulajmani,jordanlewis,awoods187,bdarnell,jseldess,RaduBerinde,Annebirzin,ricardocrdb a=knz

The following rules are applied:

- the breakdown is only displayed if the total execution time exceeds
  1ms. We assume that (human) users don't care about the breakdown if a
  statement is faster than that.

- if `verbose_timings` is unset (default), the server-side latencies
  are all grouped together (the "service latency") under a single
  `execution` label.

- the user can invoke `\set verbose_timings` to get the full
  breakdown.

Example outputs follow.

The breakdown is omitted because the total time is under 1ms:

```
[email protected]:44820/defaultdb> select 1;
  ?column?
------------
         1
(1 row)

Time: 0.000s total
```

A query is simple to parse/plan, but takes a while to execute because
it produces a lot of data:

```
[email protected]:44820/defaultdb> select generate_series(1,10000);
...
(10000 rows)

Time: 0.013s total (execution 0.004s / network 0.009s)
```

A more complex statement:

```
[email protected]:44820/defaultdb> create table t(x int);
CREATE TABLE

Time: 0.003s total (execution 0.002s / network 0.001s)
```

Release note (cli change): The display of statement timings in the SQL
shell (`cockroach sql` and `cockroach demo`) has been further simplified.

Co-authored-by: Raphael 'kena' Poss <[email protected]>

pkg/cli/auth.go

@@ -64,7 +64,7 @@ func runLogin(cmd *cobra.Command, args []string) error {
 			return err
 		}
 
-		checkInteractive()
+		checkInteractive(os.Stdin)
 		if cliCtx.isInteractive {
 			fmt.Fprintf(stderr, `#
 # Example uses:

pkg/cli/cli.go

@@ -127,10 +127,6 @@ func (e *cliError) FormatError(p errors.Printer) error {
 // to be captured.
 var stderr = log.OrigStderr
 
-// stdin aliases os.Stdin; we use an alias here so that tests in this
-// package can redirect the input of the CLI shell.
-var stdin = os.Stdin
-
 var versionCmd = &cobra.Command{
 	Use:   "version",
 	Short: "output version information",

pkg/cli/cli_test.go

@@ -2060,3 +2060,32 @@ func Example_dump_no_visible_columns() {
 	// CREATE TABLE public.t (FAMILY "primary" (rowid)
 	// );
 }
+
+// Example_read_from_file tests the -f parameter.
+// The input file contains a mix of client-side and
+// server-side commands to ensure that both are supported with -f.
+func Example_read_from_file() {
+	c := newCLITest(cliTestParams{})
+	defer c.cleanup()
+
+	c.RunWithArgs([]string{"sql", "-e", "select 1", "-f", "testdata/inputfile.sql"})
+	c.RunWithArgs([]string{"sql", "-f", "testdata/inputfile.sql"})
+
+	// Output:
+	// sql -e select 1 -f testdata/inputfile.sql
+	// ERROR: unsupported combination: --execute and --file
+	// sql -f testdata/inputfile.sql
+	// SET
+	// CREATE TABLE
+	// > INSERT INTO test(s) VALUES ('hello'), ('world');
+	// INSERT 2
+	// > SELECT * FROM test;
+	// s
+	// hello
+	// world
+	// > SELECT undefined;
+	// ERROR: column "undefined" does not exist
+	// SQLSTATE: 42703
+	// ERROR: column "undefined" does not exist
+	// SQLSTATE: 42703
+}

pkg/cli/cliflags/flags.go

@@ -256,7 +256,20 @@ Execute the SQL statement(s) on the command line, then exit. This flag may be
 specified multiple times and each value may contain multiple semicolon
 separated statements. 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 SQL statement are printed on the standard output.`,
+results of each SQL statement are printed on the standard output.
+
+This flag is incompatible with --file / -f.`,
+	}
+
+	File = FlagInfo{
+		Name:      "file",
+		Shorthand: "f",
+		Description: `
+Read and execute the SQL statement(s) from the specified file.
+The file is processed as if it has been redirected on the standard
+input of the shell.
+
+This flag is incompatible with --execute / -e.`,
 	}
 
 	Watch = FlagInfo{

pkg/cli/context.go

@@ -204,8 +204,14 @@ var sqlCtx = struct {
 	setStmts statementsValue
 
 	// execStmts is a list of statements to execute.
+	// Only valid if inputFile is empty.
 	execStmts statementsValue
 
+	// inputFile is the file to read from.
+	// If empty, os.Stdin is used.
+	// Only valid if execStmts is empty.
+	inputFile string
+
 	// repeatDelay indicates that the execStmts should be "watched"
 	// at the specified time interval. Zero disables
 	// the watch.
@@ -240,6 +246,7 @@ var sqlCtx = struct {
 func setSQLContextDefaults() {
 	sqlCtx.setStmts = nil
 	sqlCtx.execStmts = nil
+	sqlCtx.inputFile = ""
 	sqlCtx.repeatDelay = 0
 	sqlCtx.safeUpdates = false
 	sqlCtx.showTimes = false

pkg/cli/decode.go

@@ -26,13 +26,13 @@ import (
 )
 
 func runDebugDecodeProto(_ *cobra.Command, _ []string) error {
-	if isatty.IsTerminal(stdin.Fd()) {
+	if isatty.IsTerminal(os.Stdin.Fd()) {
 		fmt.Fprintln(stderr,
 			`# Reading proto-encoded pieces of data from stdin.
 # Press Ctrl+C or Ctrl+D to terminate.`,
 		)
 	}
-	return streamMap(os.Stdout, stdin,
+	return streamMap(os.Stdout, os.Stdin,
 		func(s string) (bool, string, error) { return tryDecodeValue(s, debugDecodeProtoName) })
 }
 

pkg/cli/demo.go

@@ -252,6 +252,12 @@ func checkDemoConfiguration(
 }
 
 func runDemo(cmd *cobra.Command, gen workload.Generator) (err error) {
+	cmdIn, closeFn, err := getInputFile()
+	if err != nil {
+		return err
+	}
+	defer closeFn()
+
 	if gen, err = checkDemoConfiguration(cmd, gen); err != nil {
 		return err
 	}
@@ -282,7 +288,7 @@ func runDemo(cmd *cobra.Command, gen workload.Generator) (err error) {
 	}
 	demoCtx.transientCluster = &c
 
-	checkInteractive()
+	checkInteractive(cmdIn)
 
 	if cliCtx.isInteractive {
 		fmt.Printf(`#
@@ -358,7 +364,7 @@ func runDemo(cmd *cobra.Command, gen workload.Generator) (err error) {
 	conn := makeSQLConn(c.connURL)
 	defer conn.Close()
 
-	return runClient(cmd, conn)
+	return runClient(cmd, conn, cmdIn)
 }
 
 func waitForLicense(licenseDone <-chan error) error {

pkg/cli/flags.go

@@ -656,6 +656,7 @@ func init() {
 		f := cmd.Flags()
 		varFlag(f, &sqlCtx.setStmts, cliflags.Set)
 		varFlag(f, &sqlCtx.execStmts, cliflags.Execute)
+		stringFlag(f, &sqlCtx.inputFile, cliflags.File)
 		durationFlag(f, &sqlCtx.repeatDelay, cliflags.Watch)
 		boolFlag(f, &sqlCtx.safeUpdates, cliflags.SafeUpdates)
 		boolFlag(f, &sqlCtx.debugMode, cliflags.CliDebugMode)

pkg/cli/interactive_tests/test_timing.tcl

@@ -8,10 +8,10 @@ spawn $argv demo movr
 eexpect root@
 
 start_test "Test that server execution time and network latency are printed by default."
-send "SELECT * FROM vehicles LIMIT 1;\r"
+send "SELECT pg_sleep(0.02) FROM vehicles LIMIT 1;\r"
 eexpect "1 row"
-eexpect "/ net"
-eexpect "/ other"
+eexpect "execution"
+eexpect "network"
 
 # Ditto with multiple statements on one line
 send "SELECT * FROM vehicles LIMIT 1; CREATE TABLE t(a int);\r"
@@ -21,10 +21,10 @@ end_test
 
 start_test "Test show_server_execution_times works correctly"
 send "\\set show_server_times=false\r"
-send "SELECT * FROM vehicles LIMIT 1;\r"
+send "SELECT pg_sleep(0.02) FROM vehicles LIMIT 1;\r"
 eexpect "\nTime:"
 send "\\set show_server_times=true\r"
-send "SELECT * FROM vehicles LIMIT 1;\r"
-eexpect "/ net"
-eexpect "/ other"
+send "SELECT pg_sleep(0.02) FROM vehicles LIMIT 1;\r"
+eexpect "execution"
+eexpect "network"
 end_test

pkg/cli/sql.go

@@ -1271,7 +1271,7 @@ func (c *cliState) doDecidePath() cliStateEnum {
 
 // runInteractive runs the SQL client interactively, presenting
 // a prompt to the user for each statement.
-func runInteractive(conn *sqlConn) (exitErr error) {
+func runInteractive(conn *sqlConn, cmdIn *os.File) (exitErr error) {
 	c := cliState{conn: conn}
 
 	state := cliStart
@@ -1281,7 +1281,7 @@ func runInteractive(conn *sqlConn) (exitErr error) {
 		}
 		switch state {
 		case cliStart:
-			cleanupFn, err := c.configurePreShellDefaults()
+			cleanupFn, err := c.configurePreShellDefaults(cmdIn)
 			defer cleanupFn()
 			if err != nil {
 				return err
@@ -1340,7 +1340,7 @@ func runInteractive(conn *sqlConn) (exitErr error) {
 //
 // The returned cleanupFn must be called even when the err return is
 // not nil.
-func (c *cliState) configurePreShellDefaults() (cleanupFn func(), err error) {
+func (c *cliState) configurePreShellDefaults(cmdIn *os.File) (cleanupFn func(), err error) {
 	if cliCtx.terminalOutput {
 		// If results are shown on a terminal also enable printing of
 		// times by default.
@@ -1373,11 +1373,11 @@ func (c *cliState) configurePreShellDefaults() (cleanupFn func(), err error) {
 		// the doStart() method because of the defer.
 		c.ins, c.exitErr = readline.InitFiles("cockroach",
 			true, /* wideChars */
-			stdin, os.Stdout, stderr)
+			cmdIn, os.Stdout, stderr)
 		if errors.Is(c.exitErr, readline.ErrWidecharNotSupported) {
 			log.Warning(context.TODO(), "wide character support disabled")
 			c.ins, c.exitErr = readline.InitFiles("cockroach",
-				false, stdin, os.Stdout, stderr)
+				false, cmdIn, os.Stdout, stderr)
 		}
 		if c.exitErr != nil {
 			return cleanupFn, c.exitErr
@@ -1391,7 +1391,7 @@ func (c *cliState) configurePreShellDefaults() (cleanupFn func(), err error) {
 		cleanupFn = func() { c.ins.Close() }
 	} else {
 		c.ins = noLineEditor
-		c.buf = bufio.NewReader(stdin)
+		c.buf = bufio.NewReader(cmdIn)
 		cleanupFn = func() {}
 	}
 
@@ -1476,18 +1476,41 @@ func (c *cliState) runStatements(stmts []string) error {
 
 // checkInteractive sets the isInteractive parameter depending on the
 // execution environment and the presence of -e flags.
-func checkInteractive() {
+func checkInteractive(stdin *os.File) {
 	// We don't consider sessions interactives unless we have a
 	// serious hunch they are. For now, only `cockroach sql` *without*
 	// `-e` has the ability to input from a (presumably) human user,
 	// and we'll also assume that there is no human if the standard
 	// input is not terminal-like -- likely redirected from a file,
 	// etc.
-	cliCtx.isInteractive = len(sqlCtx.execStmts) == 0 && isatty.IsTerminal(os.Stdin.Fd())
+	cliCtx.isInteractive = len(sqlCtx.execStmts) == 0 && isatty.IsTerminal(stdin.Fd())
+}
+
+// getInputFile establishes where we are reading from.
+func getInputFile() (cmdIn *os.File, closeFn func(), err error) {
+	if sqlCtx.inputFile == "" {
+		return os.Stdin, func() {}, nil
+	}
+
+	if len(sqlCtx.execStmts) != 0 {
+		return nil, nil, errors.Newf("unsupported combination: --%s and --%s", cliflags.Execute.Name, cliflags.File.Name)
+	}
+
+	f, err := os.Open(sqlCtx.inputFile)
+	if err != nil {
+		return nil, nil, err
+	}
+	return f, func() { _ = f.Close() }, nil
 }
 
 func runTerm(cmd *cobra.Command, args []string) error {
-	checkInteractive()
+	cmdIn, closeFn, err := getInputFile()
+	if err != nil {
+		return err
+	}
+	defer closeFn()
+
+	checkInteractive(cmdIn)
 
 	if cliCtx.isInteractive {
 		// The user only gets to see the welcome message on interactive sessions.
@@ -1500,10 +1523,10 @@ func runTerm(cmd *cobra.Command, args []string) error {
 	}
 	defer conn.Close()
 
-	return runClient(cmd, conn)
+	return runClient(cmd, conn, cmdIn)
 }
 
-func runClient(cmd *cobra.Command, conn *sqlConn) error {
+func runClient(cmd *cobra.Command, conn *sqlConn, cmdIn *os.File) error {
 	// Open the connection to make sure everything is OK before running any
 	// statements. Performs authentication.
 	if err := conn.ensureConn(); err != nil {
@@ -1513,7 +1536,7 @@ func runClient(cmd *cobra.Command, conn *sqlConn) error {
 	// Enable safe updates, unless disabled.
 	setupSafeUpdates(cmd, conn)
 
-	return runInteractive(conn)
+	return runInteractive(conn, cmdIn)
 }
 
 // setupSafeUpdates attempts to enable "safe mode" if the session is

pkg/cli/sql_test.go

@@ -71,7 +71,6 @@ select '''
 			f.Close()
 		}
 		_ = os.Remove(fname)
-		stdin = os.Stdin
 	}()
 
 	for _, test := range tests {
@@ -90,10 +89,7 @@ select '''
 			fmt.Fprintln(stderr, err)
 			return
 		}
-		// Override the standard input for runInteractive().
-		stdin = f
-
-		err := runInteractive(conn)
+		err := runInteractive(conn, f)
 		if err != nil {
 			fmt.Fprintln(stderr, err)
 		}