From aae816753fff212841e1bb85dae90434634006da Mon Sep 17 00:00:00 2001 From: David Shiflet Date: Wed, 16 Mar 2022 17:29:57 -0400 Subject: [PATCH 1/4] implement -r --- cmd/sqlcmd/main.go | 15 +++++++++++++++ cmd/sqlcmd/main_test.go | 4 ++++ pkg/sqlcmd/commands.go | 15 ++++++++++----- pkg/sqlcmd/commands_test.go | 22 ++++++++++++++++++++++ pkg/sqlcmd/format.go | 2 +- pkg/sqlcmd/sqlcmd.go | 26 +++++++++++++++++++------- pkg/sqlcmd/sqlcmd_test.go | 14 +++++++++++++- 7 files changed, 84 insertions(+), 14 deletions(-) diff --git a/cmd/sqlcmd/main.go b/cmd/sqlcmd/main.go index fc897e3a..c1573b04 100644 --- a/cmd/sqlcmd/main.go +++ b/cmd/sqlcmd/main.go @@ -48,6 +48,7 @@ type SQLCmdArguments struct { ErrorSeverityLevel uint8 `short:"V" help:"Controls the severity level that is used to set the ERRORLEVEL variable on exit."` ErrorLevel int `short:"m" help:"Controls which error messages are sent to stdout. Messages that have severity level greater than or equal to this level are sent."` Format string `short:"F" help:"Specifies the formatting for results." default:"horiz" enum:"horiz,horizontal,vert,vertical"` + ErrorsToStderr int `short:"r" help:"Redirects the error message output to the screen (stderr). A value of 0 means messages with severity >= 11 will b redirected. A value of 1 means all error message output including PRINT is redirected." enum:"-1,0,1" default:"-1"` } // Validate accounts for settings not described by Kong attributes @@ -220,6 +221,20 @@ func run(vars *sqlcmd.Variables, args *SQLCmdArguments) (int, error) { if err != nil { return 1, err } + } else { + var stderrSeverity uint8 = 11 + if args.ErrorsToStderr == 1 { + stderrSeverity = 0 + } + if args.ErrorsToStderr >= 0 { + s.PrintError = func(msg string, severity uint8) bool { + if severity >= stderrSeverity { + _, _ = os.Stderr.Write([]byte(msg)) + return true + } + return false + } + } } once := false if args.InitialQuery != "" { diff --git a/cmd/sqlcmd/main_test.go b/cmd/sqlcmd/main_test.go index f1cc4949..39cc0880 100644 --- a/cmd/sqlcmd/main_test.go +++ b/cmd/sqlcmd/main_test.go @@ -73,6 +73,9 @@ func TestValidCommandLineToArgsConversion(t *testing.T) { {[]string{"-F", "vert"}, func(args SQLCmdArguments) bool { return args.Format == "vert" }}, + {[]string{"-r", "1"}, func(args SQLCmdArguments) bool { + return args.ErrorsToStderr == 1 + }}, } for _, test := range commands { @@ -100,6 +103,7 @@ func TestInvalidCommandLine(t *testing.T) { // the test prefix is a kong artifact https://github.com/alecthomas/kong/issues/221 {[]string{"-a", "100"}, "test: '-a 100': Packet size has to be a number between 512 and 32767."}, {[]string{"-F", "what"}, "--format must be one of \"horiz\",\"horizontal\",\"vert\",\"vertical\" but got \"what\""}, + {[]string{"-r", "5"}, `--errors-to-stderr must be one of "-1","0","1" but got '\x05'`}, } for _, test := range commands { diff --git a/pkg/sqlcmd/commands.go b/pkg/sqlcmd/commands.go index 146c07cb..a72f70bb 100644 --- a/pkg/sqlcmd/commands.go +++ b/pkg/sqlcmd/commands.go @@ -9,7 +9,6 @@ import ( "regexp" "sort" "strings" - "syscall" "github.com/alecthomas/kong" ) @@ -188,11 +187,14 @@ func goCommand(s *Sqlcmd, args []string, line uint) error { // outCommand changes the output writer to use a file func outCommand(s *Sqlcmd, args []string, line uint) error { + if len(args) == 0 || args[0] == "" { + return InvalidCommandError("OUT", line) + } switch { case strings.EqualFold(args[0], "stdout"): - s.SetOutput(nil) + s.SetOutput(os.Stdout) case strings.EqualFold(args[0], "stderr"): - s.SetOutput(os.NewFile(uintptr(syscall.Stderr), "/dev/stderr")) + s.SetOutput(os.Stderr) default: o, err := os.OpenFile(args[0], os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0o644) if err != nil { @@ -205,11 +207,14 @@ func outCommand(s *Sqlcmd, args []string, line uint) error { // 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("OUT", line) + } switch { case strings.EqualFold(args[0], "stderr"): - s.SetError(nil) + s.SetError(os.Stderr) case strings.EqualFold(args[0], "stdout"): - s.SetError(os.NewFile(uintptr(syscall.Stderr), "/dev/stdout")) + s.SetError(os.Stdout) default: o, err := os.OpenFile(args[0], os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0o644) if err != nil { diff --git a/pkg/sqlcmd/commands_test.go b/pkg/sqlcmd/commands_test.go index 11bedd75..05426bca 100644 --- a/pkg/sqlcmd/commands_test.go +++ b/pkg/sqlcmd/commands_test.go @@ -190,3 +190,25 @@ func TestConnectCommand(t *testing.T) { } } } + +func TestErrorCommand(t *testing.T) { + s, buf := setupSqlCmdWithMemoryOutput(t) + defer buf.Close() + file, err := os.CreateTemp("", "sqlcmderr") + assert.NoError(t, err, "os.CreateTemp") + defer os.Remove(file.Name()) + fileName := file.Name() + _ = file.Close() + err = errorCommand(s, []string{""}, 1) + assert.EqualError(t, err, InvalidCommandError("OUT", 1).Error(), "errorCommand with empty file name") + err = errorCommand(s, []string{fileName}, 1) + assert.NoError(t, err, "errorCommand") + // Only some error kinds go to the error output + err = runSqlCmd(t, s, []string{"print N'message'", "RAISERROR(N'Error', 16, 1)", "SELECT 1", ":SETVAR 1", "GO"}) + assert.NoError(t, err, "runSqlCmd") + s.SetError(nil) + errText, err := os.ReadFile(file.Name()) + if assert.NoError(t, err, "ReadFile") { + assert.Equal(t, "Msg 50000, Level 16, State 1, Server DAVIDSHI-2019, Line 2"+SqlcmdEol+"Error"+SqlcmdEol, string(errText), "Error file contents") + } +} diff --git a/pkg/sqlcmd/format.go b/pkg/sqlcmd/format.go index a37ead80..a7e001e6 100644 --- a/pkg/sqlcmd/format.go +++ b/pkg/sqlcmd/format.go @@ -208,7 +208,7 @@ func (f *sqlCmdFormatterType) AddError(err error) { if print { b.WriteString(msg) b.WriteString(SqlcmdEol) - f.mustWriteOut(fitToScreen(b, f.vars.ScreenWidth()).String()) + f.mustWriteErr(fitToScreen(b, f.vars.ScreenWidth()).String()) } } diff --git a/pkg/sqlcmd/sqlcmd.go b/pkg/sqlcmd/sqlcmd.go index 6a2fa91f..ccbdb499 100644 --- a/pkg/sqlcmd/sqlcmd.go +++ b/pkg/sqlcmd/sqlcmd.go @@ -61,6 +61,8 @@ type Sqlcmd struct { Format Formatter Query string Cmd Commands + // PrintError allows the host to redirect errors away from the default output. Returns false if the error is not redirected by the host. + PrintError func(msg string, severity uint8) bool } // New creates a new Sqlcmd instance @@ -73,6 +75,9 @@ func New(l Console, workingDirectory string, vars *Variables) *Sqlcmd { } s.batch = NewBatch(s.scanNext, s.Cmd) mssql.SetContextLogger(s) + s.PrintError = func(msg string, severity uint8) bool { + return false + } return s } @@ -85,7 +90,7 @@ func (s *Sqlcmd) scanNext() (string, error) { // When processAll is true it executes any remaining batch content when reaching EOF func (s *Sqlcmd) Run(once bool, processAll bool) error { setupCloseHandler(s) - stderr, iactive := s.GetError(), s.lineIo != nil + iactive := s.lineIo != nil var lastError error for { var execute bool @@ -128,7 +133,7 @@ func (s *Sqlcmd) Run(once bool, processAll bool) error { break } if err != nil { - fmt.Fprintln(stderr, err) + _, _ = s.GetOutput().Write([]byte(err.Error() + SqlcmdEol)) lastError = err } } @@ -174,7 +179,7 @@ func (s *Sqlcmd) GetOutput() io.Writer { // SetOutput sets the io.WriteCloser to use for non-error output func (s *Sqlcmd) SetOutput(o io.WriteCloser) { - if s.out != nil { + if s.out != nil && s.out != os.Stderr && s.out != os.Stdout { s.out.Close() } s.out = o @@ -183,14 +188,14 @@ func (s *Sqlcmd) SetOutput(o io.WriteCloser) { // GetError returns the io.Writer to use for errors func (s *Sqlcmd) GetError() io.Writer { if s.err == nil { - return os.Stderr + return s.GetOutput() } return s.err } // SetError sets the io.WriteCloser to use for errors func (s *Sqlcmd) SetError(e io.WriteCloser) { - if s.err != nil { + if s.err != nil && s.err != os.Stderr && s.err != os.Stdout { s.err.Close() } s.err = e @@ -376,9 +381,16 @@ func (s *Sqlcmd) runQuery(query string) (int, error) { msg := retmsg.Message(ctx) switch m := msg.(type) { case sqlexp.MsgNotice: - s.Format.AddMessage(m.Message) + if !s.PrintError(m.Message, 10) { + s.Format.AddMessage(m.Message) + } case sqlexp.MsgError: - s.Format.AddError(m.Error) + switch e := m.Error.(type) { + case mssql.Error: + if !s.PrintError(e.Message, e.Class) { + s.Format.AddError(m.Error) + } + } qe = s.handleError(&retcode, m.Error) case sqlexp.MsgRowsAffected: if m.Count == 1 { diff --git a/pkg/sqlcmd/sqlcmd_test.go b/pkg/sqlcmd/sqlcmd_test.go index 2d91d410..f2666fa7 100644 --- a/pkg/sqlcmd/sqlcmd_test.go +++ b/pkg/sqlcmd/sqlcmd_test.go @@ -246,7 +246,7 @@ func TestSqlCmdExitOnError(t *testing.T) { err := runSqlCmd(t, s, []string{"select 1", "GO", ":setvar", "select 2", "GO"}) o := buf.buf.String() assert.EqualError(t, err, "Sqlcmd: Error: Syntax error at line 3 near command ':SETVAR'.", "Run should return an error") - assert.Equal(t, "1"+SqlcmdEol+SqlcmdEol+oneRowAffected+SqlcmdEol, o, "Only first select should run") + assert.Equal(t, "1"+SqlcmdEol+SqlcmdEol+oneRowAffected+SqlcmdEol+"Sqlcmd: Error: Syntax error at line 3 near command ':SETVAR'."+SqlcmdEol, o, "Only first select should run") assert.Equal(t, 1, s.Exitcode, "s.ExitCode for a syntax error") s, buf = setupSqlCmdWithMemoryOutput(t) @@ -396,6 +396,18 @@ func TestVerticalLayoutWithColumns(t *testing.T) { } +func TestSqlCmdDefersToPrintError(t *testing.T) { + s, buf := setupSqlCmdWithMemoryOutput(t) + defer buf.Close() + s.PrintError = func(msg string, severity uint8) bool { + return severity > 10 + } + err := runSqlCmd(t, s, []string{"PRINT 'this has severity 10'", "RAISERROR (N'Testing!' , 11, 1)", "GO"}) + if assert.NoError(t, err, "runSqlCmd failed") { + assert.Equal(t, "this has severity 10"+SqlcmdEol, buf.buf.String(), "Errors should be filtered by s.PrintError") + } +} + // runSqlCmd uses lines as input for sqlcmd instead of relying on file or console input func runSqlCmd(t testing.TB, s *Sqlcmd, lines []string) error { t.Helper() From 01d232f6ea934643562bab837e9d1c617e5e98f2 Mon Sep 17 00:00:00 2001 From: David Shiflet Date: Wed, 16 Mar 2022 17:33:59 -0400 Subject: [PATCH 2/4] update readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index aa00f9e5..90dd7f0a 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ We will be implementing command line switches and behaviors over time. Several s - The `SQLCMDPASSWORD` environment variable - The `:CONNECT` command - When prompted, the user can type the password to complete a connection (pending [#50](https://github.com/microsoft/go-sqlcmd/issues/50)) - +- `-r` requires a 0 or 1 argument - `-R` switch will be removed. The go runtime does not provide access to user locale information, and it's not readily available through syscall on all supported platforms. - `-I` switch will be removed. To disable quoted identifier behavior, add `SET QUOTED IDENTIFIER OFF` in your scripts. - `-N` now takes a string value that can be one of `true`, `false`, or `disable` to specify the encryption choice. (`default` is the same as omitting the parameter) From ec222b42f31c15de9ab41364d409df141b8af92b Mon Sep 17 00:00:00 2001 From: David Shiflet Date: Wed, 16 Mar 2022 22:29:52 -0400 Subject: [PATCH 3/4] update to go-mssqldb with a fix --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 6936235e..637ff2db 100644 --- a/go.mod +++ b/go.mod @@ -11,4 +11,4 @@ require ( github.com/stretchr/testify v1.7.0 ) -replace github.com/denisenkom/go-mssqldb => github.com/shueybubbles/go-mssqldb v0.10.1-0.20220303143659-8896461e4ec7 +replace github.com/denisenkom/go-mssqldb => github.com/shueybubbles/go-mssqldb v0.10.1-0.20220317022252-fafb9d92e469 diff --git a/go.sum b/go.sum index d909dd49..707fdc6a 100644 --- a/go.sum +++ b/go.sum @@ -27,8 +27,8 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/shueybubbles/go-mssqldb v0.10.1-0.20220303143659-8896461e4ec7 h1:4CIaYagSRCGr0/Gh6cfF5cQx3RVE3qrQukZn8iMO6Y8= -github.com/shueybubbles/go-mssqldb v0.10.1-0.20220303143659-8896461e4ec7/go.mod h1:iiK0YP1ZeepvmBQk/QpLEhhTNJgfzrpArPY/aFvc9yU= +github.com/shueybubbles/go-mssqldb v0.10.1-0.20220317022252-fafb9d92e469 h1:BuUMqsxB86i1QEBf0q+dkQYfNLVpD1nH1fRJPKvXWSg= +github.com/shueybubbles/go-mssqldb v0.10.1-0.20220317022252-fafb9d92e469/go.mod h1:iiK0YP1ZeepvmBQk/QpLEhhTNJgfzrpArPY/aFvc9yU= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= From 1705b2bceff40e1ff790c2934e1139322e7f2629 Mon Sep 17 00:00:00 2001 From: David Shiflet Date: Thu, 17 Mar 2022 15:15:38 -0400 Subject: [PATCH 4/4] remove server name from test --- pkg/sqlcmd/commands_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sqlcmd/commands_test.go b/pkg/sqlcmd/commands_test.go index 05426bca..3a0b1382 100644 --- a/pkg/sqlcmd/commands_test.go +++ b/pkg/sqlcmd/commands_test.go @@ -209,6 +209,6 @@ func TestErrorCommand(t *testing.T) { s.SetError(nil) errText, err := os.ReadFile(file.Name()) if assert.NoError(t, err, "ReadFile") { - assert.Equal(t, "Msg 50000, Level 16, State 1, Server DAVIDSHI-2019, Line 2"+SqlcmdEol+"Error"+SqlcmdEol, string(errText), "Error file contents") + assert.Regexp(t, "Msg 50000, Level 16, State 1, Server .*, Line 2"+SqlcmdEol+"Error"+SqlcmdEol, string(errText), "Error file contents") } }