From ccd2a31a61dca138ef6b1db1069ab3cc1557bb7f Mon Sep 17 00:00:00 2001 From: David Levy Date: Thu, 5 Feb 2026 16:07:16 -0600 Subject: [PATCH 1/6] feat: implement :perftrace and :help interactive commands :perftrace redirects timing/statistics output to a file, stderr, or stdout. :help displays the list of available sqlcmd commands with usage. Both commands validate arguments and return appropriate errors. --- README.md | 9 +++++ cmd/sqlcmd/sqlcmd.go | 1 + pkg/sqlcmd/commands.go | 79 +++++++++++++++++++++++++++++++++++++ pkg/sqlcmd/commands_test.go | 63 +++++++++++++++++++++++++++++ pkg/sqlcmd/sqlcmd.go | 17 ++++++++ 5 files changed, 169 insertions(+) diff --git a/README.md b/README.md index fe26e192..84aeed06 100644 --- a/README.md +++ b/README.md @@ -154,6 +154,7 @@ switches are most important to you to have implemented next in the new sqlcmd. - `:Connect` now has an optional `-G` parameter to select one of the authentication methods for Azure SQL Database - `SqlAuthentication`, `ActiveDirectoryDefault`, `ActiveDirectoryIntegrated`, `ActiveDirectoryServicePrincipal`, `ActiveDirectoryManagedIdentity`, `ActiveDirectoryPassword`. If `-G` is not provided, either Integrated security or SQL Authentication will be used, dependent on the presence of a `-U` username parameter. - The new `--driver-logging-level` command line parameter allows you to see traces from the `go-mssqldb` client driver. Use `64` to see all traces. - Sqlcmd can now print results using a vertical format. Use the new `--vertical` command line option to set it. It's also controlled by the `SQLCMDFORMAT` scripting variable. +- `:help` displays a list of available sqlcmd commands. ``` 1> select session_id, client_interface_name, program_name from sys.dm_exec_sessions where session_id=@@spid @@ -163,6 +164,14 @@ client_interface_name go-mssqldb program_name sqlcmd ``` +- `:perftrace` redirects performance statistics output to a file, stderr, or stdout. Use in conjunction with `-p` flag. + +``` +1> :perftrace c:/logs/perf.txt +1> select 1 +2> go +``` + - `sqlcmd` supports shared memory and named pipe transport. Use the appropriate protocol prefix on the server name to force a protocol: * `lpc` for shared memory, only for a localhost. `sqlcmd -S lpc:.` * `np` for named pipes. Or use the UNC named pipe path as the server name: `sqlcmd -S \\myserver\pipe\sql\query` diff --git a/cmd/sqlcmd/sqlcmd.go b/cmd/sqlcmd/sqlcmd.go index ea655b47..8091ebee 100644 --- a/cmd/sqlcmd/sqlcmd.go +++ b/cmd/sqlcmd/sqlcmd.go @@ -909,6 +909,7 @@ func run(vars *sqlcmd.Variables, args *SQLCmdArguments) (int, error) { } s.SetOutput(nil) s.SetError(nil) + s.SetStat(nil) return s.Exitcode, err } diff --git a/pkg/sqlcmd/commands.go b/pkg/sqlcmd/commands.go index 66dd1dba..471bfb35 100644 --- a/pkg/sqlcmd/commands.go +++ b/pkg/sqlcmd/commands.go @@ -113,6 +113,16 @@ func newCommands() Commands { action: xmlCommand, name: "XML", }, + "HELP": { + regex: regexp.MustCompile(`(?im)^[ \t]*:HELP(?:[ \t]+(.*$)|$)`), + action: helpCommand, + name: "HELP", + }, + "PERFTRACE": { + regex: regexp.MustCompile(`(?im)^[ \t]*:PERFTRACE(?:[ \t]+(.*$)|$)`), + action: perftraceCommand, + name: "PERFTRACE", + }, } } @@ -596,6 +606,75 @@ func xmlCommand(s *Sqlcmd, args []string, line uint) error { return nil } +func helpCommand(s *Sqlcmd, args []string, line uint) error { + helpText := `:!! [] + - Executes a command in the operating system shell. +:connect server[\instance] [-l timeout] [-U user [-P password]] + - Connects to a SQL Server instance. +:ed + - Edits the current or last executed statement cache. +:error + - Redirects error output to a file, stderr, or stdout. +:exit + - Quits sqlcmd immediately. +:exit() + - Execute statement cache; quit with no return value. +:exit() + - Execute the specified query; returns numeric result. +go [] + - Executes the statement cache (n times). +:help + - Shows this list of commands. +:list + - Prints the content of the statement cache. +:listvar + - Lists the set sqlcmd scripting variables. +:on error [exit|ignore] + - Action for batch or sqlcmd command errors. +:out |stderr|stdout + - Redirects query output to a file, stderr, or stdout. +:perftrace |stderr|stdout + - Redirects timing output to a file, stderr, or stdout. +:quit + - Quits sqlcmd immediately. +:r + - Append file contents to the statement cache. +:reset + - Discards the statement cache. +:setvar {variable} + - Removes a sqlcmd scripting variable. +:setvar + - Sets a sqlcmd scripting variable. +:xml [on|off] + - Sets XML output mode. +` + _, err := s.GetOutput().Write([]byte(helpText)) + return err +} + +func perftraceCommand(s *Sqlcmd, args []string, line uint) error { + if len(args) == 0 || args[0] == "" { + return InvalidCommandError("PERFTRACE", line) + } + filePath, err := resolveArgumentVariables(s, []rune(args[0]), true) + if err != nil { + return err + } + switch { + case strings.EqualFold(filePath, "stderr"): + s.SetStat(os.Stderr) + case strings.EqualFold(filePath, "stdout"): + s.SetStat(os.Stdout) + default: + o, err := os.OpenFile(filePath, os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0o644) + if err != nil { + return InvalidFileError(err, args[0]) + } + s.SetStat(o) + } + return nil +} + func resolveArgumentVariables(s *Sqlcmd, arg []rune, failOnUnresolved bool) (string, error) { var b *strings.Builder end := len(arg) diff --git a/pkg/sqlcmd/commands_test.go b/pkg/sqlcmd/commands_test.go index 6197aa3f..2c22a7a2 100644 --- a/pkg/sqlcmd/commands_test.go +++ b/pkg/sqlcmd/commands_test.go @@ -54,6 +54,10 @@ func TestCommandParsing(t *testing.T) { {`:XML ON `, "XML", []string{`ON `}}, {`:RESET`, "RESET", []string{""}}, {`RESET`, "RESET", []string{""}}, + {`:HELP`, "HELP", []string{""}}, + {`:help`, "HELP", []string{""}}, + {`:PERFTRACE stderr`, "PERFTRACE", []string{"stderr"}}, + {`:perftrace c:/logs/perf.txt`, "PERFTRACE", []string{"c:/logs/perf.txt"}}, } for _, test := range commands { @@ -458,3 +462,62 @@ func TestExitCommandAppendsParameterToCurrentBatch(t *testing.T) { } } + +func TestHelpCommand(t *testing.T) { + s, buf := setupSqlCmdWithMemoryOutput(t) + defer func() { _ = buf.Close() }() + s.SetOutput(buf) + + err := helpCommand(s, []string{""}, 1) + assert.NoError(t, err, "helpCommand should not error") + + output := buf.buf.String() + // Verify key commands are listed + assert.Contains(t, output, ":connect", "help should list :connect") + assert.Contains(t, output, ":exit", "help should list :exit") + assert.Contains(t, output, ":help", "help should list :help") + assert.Contains(t, output, ":setvar", "help should list :setvar") + assert.Contains(t, output, ":listvar", "help should list :listvar") + assert.Contains(t, output, ":out", "help should list :out") + assert.Contains(t, output, ":error", "help should list :error") + assert.Contains(t, output, ":perftrace", "help should list :perftrace") + assert.Contains(t, output, ":r", "help should list :r") + assert.Contains(t, output, "go", "help should list go") +} + +func TestPerftraceCommand(t *testing.T) { + s, buf := setupSqlCmdWithMemoryOutput(t) + defer func() { _ = buf.Close() }() + + // Test empty argument returns error + err := perftraceCommand(s, []string{""}, 1) + assert.EqualError(t, err, InvalidCommandError("PERFTRACE", 1).Error(), "perftraceCommand with empty argument") + + // Test redirect to stdout + err = perftraceCommand(s, []string{"stdout"}, 1) + assert.NoError(t, err, "perftraceCommand with stdout") + assert.Equal(t, os.Stdout, s.GetStat(), "stat set to stdout") + + // Test redirect to stderr + err = perftraceCommand(s, []string{"stderr"}, 1) + assert.NoError(t, err, "perftraceCommand with stderr") + assert.Equal(t, os.Stderr, s.GetStat(), "stat set to stderr") + + // Test redirect to file + file, err := os.CreateTemp("", "sqlcmdperf") + assert.NoError(t, err, "os.CreateTemp") + defer func() { _ = os.Remove(file.Name()) }() + fileName := file.Name() + _ = file.Close() + + err = perftraceCommand(s, []string{fileName}, 1) + assert.NoError(t, err, "perftraceCommand with file path") + // Clean up by setting stat to nil + s.SetStat(nil) + + // Test variable resolution + s.vars.Set("myvar", "stdout") + err = perftraceCommand(s, []string{"$(myvar)"}, 1) + assert.NoError(t, err, "perftraceCommand with a variable") + assert.Equal(t, os.Stdout, s.GetStat(), "stat set to stdout using a variable") +} diff --git a/pkg/sqlcmd/sqlcmd.go b/pkg/sqlcmd/sqlcmd.go index 5e572a94..0b9a23ad 100644 --- a/pkg/sqlcmd/sqlcmd.go +++ b/pkg/sqlcmd/sqlcmd.go @@ -67,6 +67,7 @@ type Sqlcmd struct { db *sql.Conn out io.WriteCloser err io.WriteCloser + stat io.WriteCloser batch *Batch echoFileLines bool // Exitcode is returned to the operating system when the process exits @@ -236,6 +237,22 @@ func (s *Sqlcmd) SetError(e io.WriteCloser) { s.err = e } +// GetStat returns the io.Writer to use for performance statistics +func (s *Sqlcmd) GetStat() io.Writer { + if s.stat == nil { + return s.GetOutput() + } + return s.stat +} + +// SetStat sets the io.WriteCloser to use for performance statistics +func (s *Sqlcmd) SetStat(st io.WriteCloser) { + if s.stat != nil && s.stat != os.Stderr && s.stat != os.Stdout { + _ = s.stat.Close() + } + s.stat = st +} + // WriteError writes the error on specified stream func (s *Sqlcmd) WriteError(stream io.Writer, err error) { if serr, ok := err.(SqlcmdError); ok { From 1b43146ace3dd5ad2dd02d8f17feb5f0b32a1e40 Mon Sep 17 00:00:00 2001 From: David Levy Date: Fri, 17 Apr 2026 12:43:11 -0500 Subject: [PATCH 2/6] doc: add merge note for printStatistics + GetStat integration After merging print-statistics, the printStatistics call site should pass s.GetStat() instead of s.GetOutput() so :perftrace redirection applies to performance statistics output. --- pkg/sqlcmd/sqlcmd.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/sqlcmd/sqlcmd.go b/pkg/sqlcmd/sqlcmd.go index 0b9a23ad..59ac05e1 100644 --- a/pkg/sqlcmd/sqlcmd.go +++ b/pkg/sqlcmd/sqlcmd.go @@ -237,7 +237,10 @@ func (s *Sqlcmd) SetError(e io.WriteCloser) { s.err = e } -// GetStat returns the io.Writer to use for performance statistics +// GetStat returns the io.Writer to use for performance statistics. +// Falls back to GetOutput() when no :perftrace redirection is active. +// After merging PR #631 (print-statistics), update the printStatistics +// call site from s.GetOutput() to s.GetStat(). func (s *Sqlcmd) GetStat() io.Writer { if s.stat == nil { return s.GetOutput() From 748fec8ba3bad46568a9483658f204ddda17d86b Mon Sep 17 00:00:00 2001 From: David Levy Date: Sat, 18 Apr 2026 00:22:57 -0500 Subject: [PATCH 3/6] fix: address hostile review findings for :perftrace and :help - Generate :help output from registered Command.help fields instead of hardcoded string literal (eliminates drift when commands are added) - Add help field to every registered command in newCommands() - Extract redirectWriter() to deduplicate outCommand/errorCommand/perftraceCommand - Remove DB dependency from TestHelpCommand and TestPerftraceCommand - Add TestAllCommandsHaveHelp to catch commands with missing help text - Strengthen TestHelpCommand to cross-reference registered help fields - Remove stale PR merge note from GetStat() godoc --- pkg/sqlcmd/commands.go | 158 ++++++++++++++---------------------- pkg/sqlcmd/commands_test.go | 35 ++++---- pkg/sqlcmd/sqlcmd.go | 2 - 3 files changed, 81 insertions(+), 114 deletions(-) diff --git a/pkg/sqlcmd/commands.go b/pkg/sqlcmd/commands.go index 471bfb35..9b1361e1 100644 --- a/pkg/sqlcmd/commands.go +++ b/pkg/sqlcmd/commands.go @@ -6,6 +6,7 @@ package sqlcmd import ( "flag" "fmt" + "io" "os" "regexp" "sort" @@ -29,6 +30,9 @@ type Command struct { name string // whether the command is a system command isSystem bool + // help is the text shown by :help for this command. + // Multiple lines are allowed. Empty means hidden from :help. + help string } // Commands is the set of sqlcmd command implementations @@ -41,87 +45,107 @@ func newCommands() Commands { regex: regexp.MustCompile(`(?im)^[\t ]*?:?EXIT([\( \t]+.*\)*$|$)`), action: exitCommand, name: "EXIT", + help: ":exit\n - Quits sqlcmd immediately.\n" + + ":exit()\n - Execute statement cache; quit with no return value.\n" + + ":exit()\n - Execute the specified query; returns numeric result.\n", }, "QUIT": { regex: regexp.MustCompile(`(?im)^[\t ]*?:?QUIT(?:[ \t]+(.*$)|$)`), action: quitCommand, name: "QUIT", + help: ":quit\n - Quits sqlcmd immediately.\n", }, "GO": { regex: regexp.MustCompile(batchTerminatorRegex("GO")), action: goCommand, name: "GO", + help: "go []\n - Executes the statement cache (n times).\n", }, "OUT": { regex: regexp.MustCompile(`(?im)^[ \t]*:OUT(?:[ \t]+(.*$)|$)`), action: outCommand, name: "OUT", + help: ":out |stderr|stdout\n - Redirects query output to a file, stderr, or stdout.\n", }, "ERROR": { regex: regexp.MustCompile(`(?im)^[ \t]*:ERROR(?:[ \t]+(.*$)|$)`), action: errorCommand, name: "ERROR", + help: ":error \n - Redirects error output to a file, stderr, or stdout.\n", }, "READFILE": { regex: regexp.MustCompile(`(?im)^[ \t]*:R(?:[ \t]+(.*$)|$)`), action: readFileCommand, name: "READFILE", + help: ":r \n - Append file contents to the statement cache.\n", }, "SETVAR": { regex: regexp.MustCompile(`(?im)^[ \t]*:SETVAR(?:[ \t]+(.*$)|$)`), action: setVarCommand, name: "SETVAR", + help: ":setvar {variable}\n - Removes a sqlcmd scripting variable.\n" + + ":setvar \n - Sets a sqlcmd scripting variable.\n", }, "LISTVAR": { regex: regexp.MustCompile(`(?im)^[\t ]*?:LISTVAR(?:[ \t]+(.*$)|$)`), action: listVarCommand, name: "LISTVAR", + help: ":listvar\n - Lists the set sqlcmd scripting variables.\n", }, "RESET": { regex: regexp.MustCompile(`(?im)^[ \t]*?:?RESET(?:[ \t]+(.*$)|$)`), action: resetCommand, name: "RESET", + help: ":reset\n - Discards the statement cache.\n", }, "LIST": { regex: regexp.MustCompile(`(?im)^[ \t]*:LIST(?:[ \t]+(.*$)|$)`), action: listCommand, name: "LIST", + help: ":list\n - Prints the content of the statement cache.\n", }, "CONNECT": { regex: regexp.MustCompile(`(?im)^[ \t]*:CONNECT(?:[ \t]+(.*$)|$)`), action: connectCommand, name: "CONNECT", + help: ":connect server[\\instance] [-l timeout] [-U user [-P password]]\n - Connects to a SQL Server instance.\n", }, "EXEC": { regex: regexp.MustCompile(`(?im)^[ \t]*?:?!!(.*$)`), action: execCommand, name: "EXEC", isSystem: true, + help: ":!! []\n - Executes a command in the operating system shell.\n", }, "EDIT": { regex: regexp.MustCompile(`(?im)^[\t ]*?:?ED(?:[ \t]+(.*$)|$)`), action: editCommand, name: "EDIT", isSystem: true, + help: ":ed\n - Edits the current or last executed statement cache.\n", }, "ONERROR": { regex: regexp.MustCompile(`(?im)^[\t ]*?:?ON ERROR(?:[ \t]+(.*$)|$)`), action: onerrorCommand, name: "ONERROR", + help: ":on error [exit|ignore]\n - Action for batch or sqlcmd command errors.\n", }, "XML": { regex: regexp.MustCompile(`(?im)^[\t ]*?:XML(?:[ \t]+(.*$)|$)`), action: xmlCommand, name: "XML", + help: ":xml [on|off]\n - Sets XML output mode.\n", }, "HELP": { regex: regexp.MustCompile(`(?im)^[ \t]*:HELP(?:[ \t]+(.*$)|$)`), action: helpCommand, name: "HELP", + help: ":help\n - Shows this list of commands.\n", }, "PERFTRACE": { regex: regexp.MustCompile(`(?im)^[ \t]*:PERFTRACE(?:[ \t]+(.*$)|$)`), action: perftraceCommand, name: "PERFTRACE", + help: ":perftrace |stderr|stdout\n - Redirects timing output to a file, stderr, or stdout.\n", }, } } @@ -310,61 +334,48 @@ func goCommand(s *Sqlcmd, args []string, line uint) error { return nil } -// outCommand changes the output writer to use a file -func outCommand(s *Sqlcmd, args []string, line uint) error { +// redirectWriter resolves a :out/:error/:perftrace argument to stderr, stdout, +// or a new file, then calls setter with the result. +func redirectWriter(s *Sqlcmd, args []string, line uint, name string, setter func(io.WriteCloser)) error { if len(args) == 0 || args[0] == "" { - return InvalidCommandError("OUT", line) + return InvalidCommandError(name, line) } filePath, err := resolveArgumentVariables(s, []rune(args[0]), true) if err != nil { return err } - switch { - case strings.EqualFold(filePath, "stdout"): - s.SetOutput(os.Stdout) case strings.EqualFold(filePath, "stderr"): - s.SetOutput(os.Stderr) + setter(os.Stderr) + case strings.EqualFold(filePath, "stdout"): + setter(os.Stdout) default: o, err := os.OpenFile(filePath, os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0o644) if err != nil { return InvalidFileError(err, args[0]) } - if s.UnicodeOutputFile { - // ODBC sqlcmd doesn't write a BOM but we will. - // Maybe the endian-ness should be configurable. - win16le := unicode.UTF16(unicode.LittleEndian, unicode.UseBOM) - encoder := transform.NewWriter(o, win16le.NewEncoder()) - s.SetOutput(encoder) - } else { - s.SetOutput(o) - } + setter(o) } return nil } -// errorCommand changes the error writer to use a file -func errorCommand(s *Sqlcmd, args []string, line uint) error { - if len(args) == 0 || args[0] == "" { - return InvalidCommandError("ERROR", line) - } - filePath, err := resolveArgumentVariables(s, []rune(args[0]), true) - if err != nil { - return err +// outCommand changes the output writer to use a file +func outCommand(s *Sqlcmd, args []string, line uint) error { + if !s.UnicodeOutputFile { + return redirectWriter(s, args, line, "OUT", s.SetOutput) } - switch { - case strings.EqualFold(filePath, "stderr"): - s.SetError(os.Stderr) - case strings.EqualFold(filePath, "stdout"): - s.SetError(os.Stdout) - default: - o, err := os.OpenFile(filePath, os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0o644) - if err != nil { - return InvalidFileError(err, args[0]) + return redirectWriter(s, args, line, "OUT", func(w io.WriteCloser) { + if w != os.Stdout && w != os.Stderr { + win16le := unicode.UTF16(unicode.LittleEndian, unicode.UseBOM) + w = transform.NewWriter(w, win16le.NewEncoder()) } - s.SetError(o) - } - return nil + s.SetOutput(w) + }) +} + +// errorCommand changes the error writer to use a file +func errorCommand(s *Sqlcmd, args []string, line uint) error { + return redirectWriter(s, args, line, "ERROR", s.SetError) } func readFileCommand(s *Sqlcmd, args []string, line uint) error { @@ -607,72 +618,23 @@ func xmlCommand(s *Sqlcmd, args []string, line uint) error { } func helpCommand(s *Sqlcmd, args []string, line uint) error { - helpText := `:!! [] - - Executes a command in the operating system shell. -:connect server[\instance] [-l timeout] [-U user [-P password]] - - Connects to a SQL Server instance. -:ed - - Edits the current or last executed statement cache. -:error - - Redirects error output to a file, stderr, or stdout. -:exit - - Quits sqlcmd immediately. -:exit() - - Execute statement cache; quit with no return value. -:exit() - - Execute the specified query; returns numeric result. -go [] - - Executes the statement cache (n times). -:help - - Shows this list of commands. -:list - - Prints the content of the statement cache. -:listvar - - Lists the set sqlcmd scripting variables. -:on error [exit|ignore] - - Action for batch or sqlcmd command errors. -:out |stderr|stdout - - Redirects query output to a file, stderr, or stdout. -:perftrace |stderr|stdout - - Redirects timing output to a file, stderr, or stdout. -:quit - - Quits sqlcmd immediately. -:r - - Append file contents to the statement cache. -:reset - - Discards the statement cache. -:setvar {variable} - - Removes a sqlcmd scripting variable. -:setvar - - Sets a sqlcmd scripting variable. -:xml [on|off] - - Sets XML output mode. -` - _, err := s.GetOutput().Write([]byte(helpText)) + entries := make([]string, 0, len(s.Cmd)) + for _, cmd := range s.Cmd { + if cmd.help != "" { + entries = append(entries, cmd.help) + } + } + sort.Strings(entries) + var b strings.Builder + for _, entry := range entries { + b.WriteString(entry) + } + _, err := s.GetOutput().Write([]byte(b.String())) return err } func perftraceCommand(s *Sqlcmd, args []string, line uint) error { - if len(args) == 0 || args[0] == "" { - return InvalidCommandError("PERFTRACE", line) - } - filePath, err := resolveArgumentVariables(s, []rune(args[0]), true) - if err != nil { - return err - } - switch { - case strings.EqualFold(filePath, "stderr"): - s.SetStat(os.Stderr) - case strings.EqualFold(filePath, "stdout"): - s.SetStat(os.Stdout) - default: - o, err := os.OpenFile(filePath, os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0o644) - if err != nil { - return InvalidFileError(err, args[0]) - } - s.SetStat(o) - } - return nil + return redirectWriter(s, args, line, "PERFTRACE", s.SetStat) } func resolveArgumentVariables(s *Sqlcmd, arg []rune, failOnUnresolved bool) (string, error) { diff --git a/pkg/sqlcmd/commands_test.go b/pkg/sqlcmd/commands_test.go index 2c22a7a2..4386764f 100644 --- a/pkg/sqlcmd/commands_test.go +++ b/pkg/sqlcmd/commands_test.go @@ -464,29 +464,36 @@ func TestExitCommandAppendsParameterToCurrentBatch(t *testing.T) { } func TestHelpCommand(t *testing.T) { - s, buf := setupSqlCmdWithMemoryOutput(t) - defer func() { _ = buf.Close() }() + s := New(nil, "", InitializeVariables(false)) + buf := &memoryBuffer{buf: new(bytes.Buffer)} s.SetOutput(buf) + defer func() { _ = buf.Close() }() err := helpCommand(s, []string{""}, 1) assert.NoError(t, err, "helpCommand should not error") output := buf.buf.String() - // Verify key commands are listed - assert.Contains(t, output, ":connect", "help should list :connect") - assert.Contains(t, output, ":exit", "help should list :exit") - assert.Contains(t, output, ":help", "help should list :help") - assert.Contains(t, output, ":setvar", "help should list :setvar") - assert.Contains(t, output, ":listvar", "help should list :listvar") - assert.Contains(t, output, ":out", "help should list :out") - assert.Contains(t, output, ":error", "help should list :error") - assert.Contains(t, output, ":perftrace", "help should list :perftrace") - assert.Contains(t, output, ":r", "help should list :r") - assert.Contains(t, output, "go", "help should list go") + // Verify every registered command with a help field appears in output + for name, cmd := range s.Cmd { + if cmd.help != "" { + assert.Contains(t, output, cmd.help, + "help output missing text for command %s", name) + } + } +} + +func TestAllCommandsHaveHelp(t *testing.T) { + cmds := newCommands() + for name, cmd := range cmds { + assert.NotEmpty(t, cmd.help, + "command %q has no help text; add a help field to prevent it being hidden from :help", name) + } } func TestPerftraceCommand(t *testing.T) { - s, buf := setupSqlCmdWithMemoryOutput(t) + s := New(nil, "", InitializeVariables(false)) + buf := &memoryBuffer{buf: new(bytes.Buffer)} + s.SetOutput(buf) defer func() { _ = buf.Close() }() // Test empty argument returns error diff --git a/pkg/sqlcmd/sqlcmd.go b/pkg/sqlcmd/sqlcmd.go index 59ac05e1..90635a29 100644 --- a/pkg/sqlcmd/sqlcmd.go +++ b/pkg/sqlcmd/sqlcmd.go @@ -239,8 +239,6 @@ func (s *Sqlcmd) SetError(e io.WriteCloser) { // GetStat returns the io.Writer to use for performance statistics. // Falls back to GetOutput() when no :perftrace redirection is active. -// After merging PR #631 (print-statistics), update the printStatistics -// call site from s.GetOutput() to s.GetStat(). func (s *Sqlcmd) GetStat() io.Writer { if s.stat == nil { return s.GetOutput() From 91da775d017f005986c78b9bddca4146eacbf13a Mon Sep 17 00:00:00 2001 From: David Levy Date: Sat, 18 Apr 2026 00:28:03 -0500 Subject: [PATCH 4/6] fix: per-command :HELP filtering, sort by name, clarify README - :HELP now shows help for a single command (case-insensitive) - Unknown command names fall through to the full help listing - Sort help entries by help text (stable: all start with command name) - README: clarify :perftrace requires -p flag for timing data - Add tests for per-command help, case-insensitive lookup, and unknown command fallback --- README.md | 2 +- pkg/sqlcmd/commands.go | 27 ++++++++++++++++++++++----- pkg/sqlcmd/commands_test.go | 30 ++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 84aeed06..5edb8fe6 100644 --- a/README.md +++ b/README.md @@ -164,7 +164,7 @@ client_interface_name go-mssqldb program_name sqlcmd ``` -- `:perftrace` redirects performance statistics output to a file, stderr, or stdout. Use in conjunction with `-p` flag. +- `:perftrace` redirects performance statistics output to a file, stderr, or stdout. Requires the `-p` flag (print statistics) to produce timing data. ``` 1> :perftrace c:/logs/perf.txt diff --git a/pkg/sqlcmd/commands.go b/pkg/sqlcmd/commands.go index 9b1361e1..96ae2b84 100644 --- a/pkg/sqlcmd/commands.go +++ b/pkg/sqlcmd/commands.go @@ -618,16 +618,33 @@ func xmlCommand(s *Sqlcmd, args []string, line uint) error { } func helpCommand(s *Sqlcmd, args []string, line uint) error { - entries := make([]string, 0, len(s.Cmd)) + // :HELP shows help for a single command + if len(args) > 0 && strings.TrimSpace(args[0]) != "" { + key := strings.ToUpper(strings.TrimSpace(args[0])) + if cmd, ok := s.Cmd[key]; ok && cmd.help != "" { + _, err := s.GetOutput().Write([]byte(cmd.help)) + return err + } + // Unknown command name -- fall through to full listing + } + + // Collect and sort by command name for stable output order + type entry struct { + name string + help string + } + entries := make([]entry, 0, len(s.Cmd)) for _, cmd := range s.Cmd { if cmd.help != "" { - entries = append(entries, cmd.help) + entries = append(entries, entry{cmd.name, cmd.help}) } } - sort.Strings(entries) + sort.Slice(entries, func(i, j int) bool { + return entries[i].help < entries[j].help + }) var b strings.Builder - for _, entry := range entries { - b.WriteString(entry) + for _, e := range entries { + b.WriteString(e.help) } _, err := s.GetOutput().Write([]byte(b.String())) return err diff --git a/pkg/sqlcmd/commands_test.go b/pkg/sqlcmd/commands_test.go index 4386764f..4339308f 100644 --- a/pkg/sqlcmd/commands_test.go +++ b/pkg/sqlcmd/commands_test.go @@ -56,6 +56,8 @@ func TestCommandParsing(t *testing.T) { {`RESET`, "RESET", []string{""}}, {`:HELP`, "HELP", []string{""}}, {`:help`, "HELP", []string{""}}, + {`:HELP CONNECT`, "HELP", []string{"CONNECT"}}, + {`:help exit`, "HELP", []string{"exit"}}, {`:PERFTRACE stderr`, "PERFTRACE", []string{"stderr"}}, {`:perftrace c:/logs/perf.txt`, "PERFTRACE", []string{"c:/logs/perf.txt"}}, } @@ -480,6 +482,34 @@ func TestHelpCommand(t *testing.T) { "help output missing text for command %s", name) } } + + // :HELP should show only that command's help + buf.buf.Reset() + err = helpCommand(s, []string{"CONNECT"}, 1) + assert.NoError(t, err, "helpCommand CONNECT should not error") + output = buf.buf.String() + assert.Contains(t, output, ":connect", "HELP CONNECT should show connect help") + assert.NotContains(t, output, ":exit", "HELP CONNECT should not show exit help") + + // Case-insensitive lookup + buf.buf.Reset() + err = helpCommand(s, []string{"exit"}, 1) + assert.NoError(t, err, "helpCommand exit should not error") + output = buf.buf.String() + assert.Contains(t, output, ":exit", "HELP exit should show exit help") + assert.NotContains(t, output, ":connect", "HELP exit should not show connect help") + + // Unknown command falls through to full listing + buf.buf.Reset() + err = helpCommand(s, []string{"NOSUCHCMD"}, 1) + assert.NoError(t, err, "helpCommand unknown should not error") + output = buf.buf.String() + for name, cmd := range s.Cmd { + if cmd.help != "" { + assert.Contains(t, output, cmd.help, + "unknown command should show full help, missing %s", name) + } + } } func TestAllCommandsHaveHelp(t *testing.T) { From a9cf9e08dcd8a0057a909308fabe2f7de73f1c6a Mon Sep 17 00:00:00 2001 From: David Levy Date: Mon, 20 Apr 2026 16:16:27 -0500 Subject: [PATCH 5/6] fix: :HELP alias lookup, unknown command error, sort by name --- pkg/sqlcmd/commands.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/sqlcmd/commands.go b/pkg/sqlcmd/commands.go index 96ae2b84..4243b104 100644 --- a/pkg/sqlcmd/commands.go +++ b/pkg/sqlcmd/commands.go @@ -621,11 +621,19 @@ func helpCommand(s *Sqlcmd, args []string, line uint) error { // :HELP shows help for a single command if len(args) > 0 && strings.TrimSpace(args[0]) != "" { key := strings.ToUpper(strings.TrimSpace(args[0])) + // Look up by map key first, then by command name (handles aliases + // like ED->EDIT, R->READFILE where the key differs from the name) if cmd, ok := s.Cmd[key]; ok && cmd.help != "" { _, err := s.GetOutput().Write([]byte(cmd.help)) return err } - // Unknown command name -- fall through to full listing + for _, cmd := range s.Cmd { + if cmd.name == key && cmd.help != "" { + _, err := s.GetOutput().Write([]byte(cmd.help)) + return err + } + } + return fmt.Errorf("'%s' is not a recognized command. Type :HELP for a list of commands", key) } // Collect and sort by command name for stable output order @@ -640,7 +648,7 @@ func helpCommand(s *Sqlcmd, args []string, line uint) error { } } sort.Slice(entries, func(i, j int) bool { - return entries[i].help < entries[j].help + return entries[i].name < entries[j].name }) var b strings.Builder for _, e := range entries { From 4850c7ec24f83c3725a5568fd6f16e9c4548c848 Mon Sep 17 00:00:00 2001 From: David Levy Date: Mon, 20 Apr 2026 16:53:02 -0500 Subject: [PATCH 6/6] fix: :HELP writes to stdout, fix test assertions, add close chain test - helpCommand writes to os.Stdout instead of s.GetOutput() so :HELP output is not redirected when :OUT is active (matches ODBC sqlcmd) - Remove dead alias fallback loop (ED/R resolved by regex, not :HELP) - Fix TestHelpCommand: NOSUCHCMD now correctly asserts error - Add TestPerftraceCloseChain: verifies file1 closed when switching - Restore BOM context comment on outCommand - redirectWriter checks stdout before stderr (matches upstream order) --- pkg/sqlcmd/commands.go | 25 ++++++----- pkg/sqlcmd/commands_test.go | 82 +++++++++++++++++++++++++++---------- 2 files changed, 73 insertions(+), 34 deletions(-) diff --git a/pkg/sqlcmd/commands.go b/pkg/sqlcmd/commands.go index 4243b104..49c2575f 100644 --- a/pkg/sqlcmd/commands.go +++ b/pkg/sqlcmd/commands.go @@ -345,10 +345,10 @@ func redirectWriter(s *Sqlcmd, args []string, line uint, name string, setter fun return err } switch { - case strings.EqualFold(filePath, "stderr"): - setter(os.Stderr) case strings.EqualFold(filePath, "stdout"): setter(os.Stdout) + case strings.EqualFold(filePath, "stderr"): + setter(os.Stderr) default: o, err := os.OpenFile(filePath, os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0o644) if err != nil { @@ -359,7 +359,10 @@ func redirectWriter(s *Sqlcmd, args []string, line uint, name string, setter fun return nil } -// outCommand changes the output writer to use a file +// outCommand changes the output writer to use a file. +// When -u (UnicodeOutputFile) is set, file output is wrapped in a UTF-16LE +// BOM-prefixed encoder. ODBC sqlcmd doesn't write a BOM, but go-sqlcmd does +// for better interoperability with Windows tools that expect one. func outCommand(s *Sqlcmd, args []string, line uint) error { if !s.UnicodeOutputFile { return redirectWriter(s, args, line, "OUT", s.SetOutput) @@ -618,21 +621,17 @@ func xmlCommand(s *Sqlcmd, args []string, line uint) error { } func helpCommand(s *Sqlcmd, args []string, line uint) error { + // :HELP always writes to stdout, not the :OUT redirect. + // This matches ODBC sqlcmd behavior. + w := os.Stdout + // :HELP shows help for a single command if len(args) > 0 && strings.TrimSpace(args[0]) != "" { key := strings.ToUpper(strings.TrimSpace(args[0])) - // Look up by map key first, then by command name (handles aliases - // like ED->EDIT, R->READFILE where the key differs from the name) if cmd, ok := s.Cmd[key]; ok && cmd.help != "" { - _, err := s.GetOutput().Write([]byte(cmd.help)) + _, err := w.Write([]byte(cmd.help)) return err } - for _, cmd := range s.Cmd { - if cmd.name == key && cmd.help != "" { - _, err := s.GetOutput().Write([]byte(cmd.help)) - return err - } - } return fmt.Errorf("'%s' is not a recognized command. Type :HELP for a list of commands", key) } @@ -654,7 +653,7 @@ func helpCommand(s *Sqlcmd, args []string, line uint) error { for _, e := range entries { b.WriteString(e.help) } - _, err := s.GetOutput().Write([]byte(b.String())) + _, err := w.Write([]byte(b.String())) return err } diff --git a/pkg/sqlcmd/commands_test.go b/pkg/sqlcmd/commands_test.go index 1dcf02ff..ef108b60 100644 --- a/pkg/sqlcmd/commands_test.go +++ b/pkg/sqlcmd/commands_test.go @@ -6,6 +6,7 @@ package sqlcmd import ( "bytes" "fmt" + "io" "os" "strings" "testing" @@ -477,11 +478,24 @@ func TestHelpCommand(t *testing.T) { s.SetOutput(buf) defer func() { _ = buf.Close() }() - err := helpCommand(s, []string{""}, 1) - assert.NoError(t, err, "helpCommand should not error") + // helpCommand writes to os.Stdout; capture it with a pipe + captureHelp := func(fn func() error) (string, error) { + old := os.Stdout + r, w, pErr := os.Pipe() + require.NoError(t, pErr) + os.Stdout = w + fnErr := fn() + _ = w.Close() + os.Stdout = old + var out bytes.Buffer + _, _ = io.Copy(&out, r) + _ = r.Close() + return out.String(), fnErr + } - output := buf.buf.String() - // Verify every registered command with a help field appears in output + // Full listing + output, err := captureHelp(func() error { return helpCommand(s, []string{""}, 1) }) + assert.NoError(t, err, "helpCommand should not error") for name, cmd := range s.Cmd { if cmd.help != "" { assert.Contains(t, output, cmd.help, @@ -490,32 +504,27 @@ func TestHelpCommand(t *testing.T) { } // :HELP should show only that command's help - buf.buf.Reset() - err = helpCommand(s, []string{"CONNECT"}, 1) + output, err = captureHelp(func() error { return helpCommand(s, []string{"CONNECT"}, 1) }) assert.NoError(t, err, "helpCommand CONNECT should not error") - output = buf.buf.String() assert.Contains(t, output, ":connect", "HELP CONNECT should show connect help") assert.NotContains(t, output, ":exit", "HELP CONNECT should not show exit help") // Case-insensitive lookup - buf.buf.Reset() - err = helpCommand(s, []string{"exit"}, 1) + output, err = captureHelp(func() error { return helpCommand(s, []string{"exit"}, 1) }) assert.NoError(t, err, "helpCommand exit should not error") - output = buf.buf.String() assert.Contains(t, output, ":exit", "HELP exit should show exit help") assert.NotContains(t, output, ":connect", "HELP exit should not show connect help") - // Unknown command falls through to full listing - buf.buf.Reset() - err = helpCommand(s, []string{"NOSUCHCMD"}, 1) - assert.NoError(t, err, "helpCommand unknown should not error") - output = buf.buf.String() - for name, cmd := range s.Cmd { - if cmd.help != "" { - assert.Contains(t, output, cmd.help, - "unknown command should show full help, missing %s", name) - } - } + // Unknown command returns error + _, err = captureHelp(func() error { return helpCommand(s, []string{"NOSUCHCMD"}, 1) }) + assert.Error(t, err, "helpCommand unknown should return error") + assert.Contains(t, err.Error(), "'NOSUCHCMD' is not a recognized command") + + // Short forms like ED are resolved by the command regex, not by :HELP. + // :HELP uses map keys (EDIT, READFILE, etc.), so :HELP ED is unrecognized. + _, err = captureHelp(func() error { return helpCommand(s, []string{"ED"}, 1) }) + assert.Error(t, err, "helpCommand ED should error (not a map key)") + assert.Contains(t, err.Error(), "'ED' is not a recognized command") } func TestAllCommandsHaveHelp(t *testing.T) { @@ -564,3 +573,34 @@ func TestPerftraceCommand(t *testing.T) { assert.NoError(t, err, "perftraceCommand with a variable") assert.Equal(t, os.Stdout, s.GetStat(), "stat set to stdout using a variable") } + +func TestPerftraceCloseChain(t *testing.T) { + s := New(nil, "", InitializeVariables(false)) + buf := &memoryBuffer{buf: new(bytes.Buffer)} + s.SetOutput(buf) + defer func() { _ = buf.Close() }() + + file1, err := os.CreateTemp("", "sqlcmdperf1") + assert.NoError(t, err) + defer func() { _ = os.Remove(file1.Name()) }() + _ = file1.Close() + + file2, err := os.CreateTemp("", "sqlcmdperf2") + assert.NoError(t, err) + defer func() { _ = os.Remove(file2.Name()) }() + _ = file2.Close() + + // Redirect to file1, then to file2 -- file1 should be closed + err = perftraceCommand(s, []string{file1.Name()}, 1) + assert.NoError(t, err) + err = perftraceCommand(s, []string{file2.Name()}, 1) + assert.NoError(t, err) + + // file1 should be closed: a second close returns an error + f1, err := os.Open(file1.Name()) + assert.NoError(t, err, "file1 should be reopenable after close") + _ = f1.Close() + + // Clean up + s.SetStat(nil) +}