Merge #130736

130736: cli: CLI prints out previous error message when executing a quit command r=rafiss a=hanpen24 Previously, when executing the quit command in the CLI, the last SQL error message would be displayed again. This was inadequate because there is no need to show a previous SQL error when executing a quit command. To address this, this patch modifies the behavior so that when commands like quit or \q are executed, any previous error messages are cleared. Fixes: #85613 Release note (bug fix): Fixed a bug where the CLI would print out the previous error message when executing the quit command. Co-authored-by: hanpen24 <[email protected]>
cockroachdb · Oct 14, 2024 · 9e12f67 · 9e12f67
2 parents a0f39e7 + f3c5b1f
commit 9e12f67
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 2 deletions.
diff --git a/pkg/cli/clisqlshell/sql.go b/pkg/cli/clisqlshell/sql.go
@@ -1267,6 +1267,8 @@ func (c *cliState) doProcessFirstLine(startState, nextState cliStateEnum) cliSta
 		return startState
 
 	case "exit", "quit":
+		// When explicitly exiting, clear exitErr.
+		c.exitErr = nil
 		return cliStop
 	}
 
@@ -1412,6 +1414,8 @@ func (c *cliState) doHandleCliCmd(loopState, nextState cliStateEnum) cliStateEnu
 
 	switch cmd[0] {
 	case `\q`, `\quit`, `\exit`:
+		// When explicitly exiting, clear exitErr.
+		c.exitErr = nil
 		return cliStop
 
 	case `\`, `\?`, `\help`:

diff --git a/pkg/cli/clisqlshell/sql_test.go b/pkg/cli/clisqlshell/sql_test.go
@@ -55,6 +55,12 @@ func Example_sql() {
 	c.RunWithArgs([]string{`sql`, `-e`, `select 1/(i-2) from generate_series(1,3) g(i)`})
 	c.RunWithArgs([]string{`sql`, `-e`, `SELECT '20:01:02+03:04:05'::timetz AS regression_65066`})
 
+	// Check that previous SQL error message is not displayed when the CLI is exited.
+	c.RunWithArgs([]string{`sql`, `-e`, `SELECT 1 FROM hoge`})
+	c.RunWithArgs([]string{`sql`, `-e`, `exit`})
+	c.RunWithArgs([]string{`sql`, `-e`, `SELECT 1 FROM hoge`})
+	c.RunWithArgs([]string{`sql`, `-e`, `\q`})
+
 	// Output:
 	// sql -e show application_name
 	// application_name
@@ -113,6 +119,14 @@ func Example_sql() {
 	// sql -e SELECT '20:01:02+03:04:05'::timetz AS regression_65066
 	// regression_65066
 	// 20:01:02+03:04:05
+	// sql -e SELECT 1 FROM hoge
+	// ERROR: relation "hoge" does not exist
+	// SQLSTATE: 42P01
+	// sql -e exit
+	// sql -e SELECT 1 FROM hoge
+	// ERROR: relation "hoge" does not exist
+	// SQLSTATE: 42P01
+	// sql -e \q
 }
 
 func Example_sql_config() {

diff --git a/pkg/cli/interactive_tests/test_last_statement.tcl b/pkg/cli/interactive_tests/test_last_statement.tcl
@@ -11,14 +11,14 @@ eexpect ":/# "
 send "$argv sql --no-line-editor\r"
 eexpect root@
 
-start_test "Check that an error in the last statement is propagated to the shell."
+start_test "Even if the last statement has a syntax error, if the user explicitly exits, the shell status should be normal exit."
 send "select ++;\r"
 eexpect "syntax error"
 eexpect root@
 send "\\q\r"
 eexpect ":/# "
 send "echo hello \$?\r"
-eexpect "hello 1"
+eexpect "hello 0"
 eexpect ":/# "
 end_test