diff --git a/CHANGELOG.md b/CHANGELOG.md index dfa654211..0531ea279 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,11 +9,26 @@ newer. Previously PostgreSQL 8.4 and newer were supported. - Add support for NamedValueChecker interface ([#1125]) +- The `pq.Error.ErrorWithDetail()` method prints a more detailed multiline + message, with the Detail, Hint, and error position (if any) ([#1219]): + + ERROR: syntax error at or near ")" (42601) + CONTEXT: line 12, column 1: + + 10 | name varchar, + 11 | version varchar, + 12 | ); + ^ + ### Fixes - Match HOME directory lookup logic with libpq: prefer $HOME over /etc/passwd, ignore ENOTDIR errors, and use APPDATA on Windows ([#1214]). +- The `pq.Error.Error()` message now includes the SQLSTATE error code ([#1219]): + + pq: syntax error at end of input (42601) + - Fix build with wasm ([#1184]), appengine ([#745]), and Plan 9 ([#1133]). - Deprecate and type alias `pq.NullTime` to `sql.NullTime` ([#1211]). @@ -47,6 +62,7 @@ newer. Previously PostgreSQL 8.4 and newer were supported. [#1211]: https://github.com/lib/pq/pull/1211 [#1212]: https://github.com/lib/pq/pull/1212 [#1214]: https://github.com/lib/pq/pull/1214 +[#1219]: https://github.com/lib/pq/pull/1219 v1.10.9 (2023-04-26) diff --git a/conn.go b/conn.go index 5d344c2a6..738125684 100644 --- a/conn.go +++ b/conn.go @@ -717,7 +717,7 @@ func (cn *conn) simpleExec(q string) (res driver.Result, commandTag string, err // done return case 'E': - err = parseError(r) + err = parseError(r, q) case 'I': res = emptyRows case 'T', 'D': @@ -730,7 +730,7 @@ func (cn *conn) simpleExec(q string) (res driver.Result, commandTag string, err } func (cn *conn) simpleQuery(q string) (res *rows, err error) { - defer cn.errRecover(&err) + defer cn.errRecover(&err, q) b := cn.writeBuf('Q') b.string(q) @@ -769,7 +769,7 @@ func (cn *conn) simpleQuery(q string) (res *rows, err error) { return case 'E': res = nil - err = parseError(r) + err = parseError(r, q) case 'D': if res == nil { cn.err.set(driver.ErrBadConn) @@ -886,7 +886,7 @@ func (cn *conn) Prepare(q string) (_ driver.Stmt, err error) { if err := cn.err.get(); err != nil { return nil, err } - defer cn.errRecover(&err) + defer cn.errRecover(&err, q) if len(q) >= 4 && strings.EqualFold(q[:4], "COPY") { s, err := cn.prepareCopyIn(q) @@ -928,7 +928,7 @@ func (cn *conn) query(query string, args []driver.Value) (_ *rows, err error) { if cn.inCopy { return nil, errCopyInProgress } - defer cn.errRecover(&err) + defer cn.errRecover(&err, query) // Check to see if we can use the "simpleQuery" interface, which is // *much* faster than going through prepare/exec @@ -959,7 +959,7 @@ func (cn *conn) Exec(query string, args []driver.Value) (res driver.Result, err if err := cn.err.get(); err != nil { return nil, err } - defer cn.errRecover(&err) + defer cn.errRecover(&err, query) // Check to see if we can use the "simpleExec" interface, which is // *much* faster than going through prepare/exec @@ -1084,10 +1084,10 @@ func (cn *conn) recv() (t byte, r *readBuf) { } switch t { case 'E': - panic(parseError(r)) + panic(parseError(r, "")) case 'N': if n := cn.noticeHandler; n != nil { - n(parseError(r)) + n(parseError(r, "")) } case 'A': if n := cn.notificationHandler; n != nil { @@ -1115,7 +1115,7 @@ func (cn *conn) recv1Buf(r *readBuf) byte { } case 'N': if n := cn.noticeHandler; n != nil { - n(parseError(r)) + n(parseError(r, "")) } case 'S': cn.processParameterStatus(r) @@ -1619,7 +1619,7 @@ func (rs *rows) Next(dest []driver.Value) (err error) { t := conn.recv1Buf(&rs.rb) switch t { case 'E': - err = parseError(&rs.rb) + err = parseError(&rs.rb, "") case 'C', 'I': if t == 'C' { rs.result, rs.tag = conn.parseComplete(rs.rb.string()) @@ -1857,7 +1857,7 @@ func (cn *conn) readParseResponse() { case '1': return case 'E': - err := parseError(r) + err := parseError(r, "") cn.readReadyForQuery() panic(err) default: @@ -1886,7 +1886,7 @@ func (cn *conn) readStatementDescribeResponse() ( colNames, colTyps = parseStatementRowDescribe(r) return paramTyps, colNames, colTyps case 'E': - err := parseError(r) + err := parseError(r, "") cn.readReadyForQuery() panic(err) default: @@ -1904,7 +1904,7 @@ func (cn *conn) readPortalDescribeResponse() rowsHeader { case 'n': return rowsHeader{} case 'E': - err := parseError(r) + err := parseError(r, "") cn.readReadyForQuery() panic(err) default: @@ -1920,7 +1920,7 @@ func (cn *conn) readBindResponse() { case '2': return case 'E': - err := parseError(r) + err := parseError(r, "") cn.readReadyForQuery() panic(err) default: @@ -1943,7 +1943,7 @@ func (cn *conn) postExecuteWorkaround() { t, r := cn.recv1() switch t { case 'E': - err := parseError(r) + err := parseError(r, "") cn.readReadyForQuery() panic(err) case 'C', 'D', 'I': @@ -1958,9 +1958,7 @@ func (cn *conn) postExecuteWorkaround() { } // Only for Exec(), since we ignore the returned data -func (cn *conn) readExecuteResponse( - protocolState string, -) (res driver.Result, commandTag string, err error) { +func (cn *conn) readExecuteResponse(protocolState string) (res driver.Result, commandTag string, err error) { for { t, r := cn.recv1() switch t { @@ -1977,7 +1975,7 @@ func (cn *conn) readExecuteResponse( } return res, commandTag, err case 'E': - err = parseError(r) + err = parseError(r, "") case 'T', 'D', 'I': if err != nil { cn.err.set(driver.ErrBadConn) diff --git a/copy.go b/copy.go index 473c953d7..92fc05f07 100644 --- a/copy.go +++ b/copy.go @@ -106,7 +106,7 @@ awaitCopyInResponse: err = errCopyToNotSupported break awaitCopyInResponse case 'E': - err = parseError(r) + err = parseError(r, q) case 'Z': if err == nil { ci.setBad(driver.ErrBadConn) @@ -170,14 +170,14 @@ func (ci *copyin) resploop() { ci.setResult(res) case 'N': if n := ci.cn.noticeHandler; n != nil { - n(parseError(&r)) + n(parseError(&r, "")) } case 'Z': ci.cn.processReadyForQuery(&r) ci.done <- true return case 'E': - err := parseError(&r) + err := parseError(&r, "") ci.setError(err) default: ci.setBad(driver.ErrBadConn) diff --git a/error.go b/error.go index 0b56b1eb6..502716772 100644 --- a/error.go +++ b/error.go @@ -6,6 +6,9 @@ import ( "io" "net" "runtime" + "strconv" + "strings" + "unicode/utf8" ) // Error severities @@ -21,25 +24,99 @@ const ( // Error represents an error communicating with the server. // +// The [Error] method only returns the error message and error code: +// +// pq: invalid input syntax for type json (22P02) +// +// The [ErrorWithDetail] method also includes the error Detail, Hint, and +// location context (if any): +// +// ERROR: invalid input syntax for type json (22P02) +// DETAIL: Token "asd" is invalid. +// CONTEXT: line 5, column 8: +// +// 3 | 'def', +// 4 | 123, +// 5 | 'foo', 'asd'::jsonb +// ^ +// // See http://www.postgresql.org/docs/current/static/protocol-error-fields.html for details of the fields type Error struct { - Severity string - Code ErrorCode - Message string - Detail string - Hint string - Position string + // [Efatal], [Epanic], [Ewarning], [Enotice], [Edebug], [Einfo], or [Elog]. + // Always present. + Severity string + + // SQLSTATE code. Always present. + Code ErrorCode + + // Primary human-readable error message. This should be accurate but terse + // (typically one line). Always present. + Message string + + // Optional secondary error message carrying more detail about the problem. + // Might run to multiple lines. + Detail string + + // Optional suggestion what to do about the problem. This is intended to + // differ from Detail in that it offers advice (potentially inappropriate) + // rather than hard facts. Might run to multiple lines. + Hint string + + // error position as an index into the original query string, as decimal + // ASCII integer. The first character has index 1, and positions are + // measured in characters not bytes. + Position string + + // This is defined the same as the Position field, but it is used when the + // cursor position refers to an internally generated command rather than the + // one submitted by the client. The InternalQuery field will always appear + // when this field appears. InternalPosition string - InternalQuery string - Where string - Schema string - Table string - Column string - DataTypeName string - Constraint string - File string - Line string - Routine string + + // Text of a failed internally-generated command. This could be, for + // example, an SQL query issued by a PL/pgSQL function. + InternalQuery string + + // An indication of the context in which the error occurred. Presently this + // includes a call stack traceback of active procedural language functions + // and internally-generated queries. The trace is one entry per line, most + // recent first. + Where string + + // If the error was associated with a specific database object, the name of + // the schema containing that object, if any. + Schema string + + // If the error was associated with a specific table, the name of the table. + // (Refer to the schema name field for the name of the table's schema.) + Table string + + // If the error was associated with a specific table column, the name of the + // column. (Refer to the schema and table name fields to identify the + // table.) + Column string + + // If the error was associated with a specific data type, the name of the + // data type. (Refer to the schema name field for the name of the data + // type's schema.) + DataTypeName string + + // If the error was associated with a specific constraint, the name of the + // constraint. Refer to fields listed above for the associated table or + // domain. (For this purpose, indexes are treated as constraints, even if + // they weren't created with constraint syntax.) + Constraint string + + // File name of the source-code location where the error was reported. + File string + + // Line number of the source-code location where the error was reported. + Line string + + // Name of the source-code routine reporting the error. + Routine string + + query string } // ErrorCode is a five-character error code. @@ -353,8 +430,8 @@ var errorCodeNames = map[ErrorCode]string{ "XX002": "index_corrupted", } -func parseError(r *readBuf) *Error { - err := new(Error) +func parseError(r *readBuf, q string) *Error { + err := &Error{query: q} for t := r.byte(); t != 0; t = r.byte() { msg := r.string() switch t { @@ -398,63 +475,121 @@ func parseError(r *readBuf) *Error { } // Fatal returns true if the Error Severity is fatal. -func (err *Error) Fatal() bool { - return err.Severity == Efatal +func (e *Error) Fatal() bool { + return e.Severity == Efatal } // SQLState returns the SQLState of the error. -func (err *Error) SQLState() string { - return string(err.Code) +func (e *Error) SQLState() string { + return string(e.Code) } -// Get implements the legacy PGError interface. New code should use the fields -// of the Error struct directly. -func (err *Error) Get(k byte) (v string) { +// Get implements the legacy PGError interface. +// +// Deprecated: new code should use the fields of the Error struct directly. +func (e *Error) Get(k byte) (v string) { switch k { case 'S': - return err.Severity + return e.Severity case 'C': - return string(err.Code) + return string(e.Code) case 'M': - return err.Message + return e.Message case 'D': - return err.Detail + return e.Detail case 'H': - return err.Hint + return e.Hint case 'P': - return err.Position + return e.Position case 'p': - return err.InternalPosition + return e.InternalPosition case 'q': - return err.InternalQuery + return e.InternalQuery case 'W': - return err.Where + return e.Where case 's': - return err.Schema + return e.Schema case 't': - return err.Table + return e.Table case 'c': - return err.Column + return e.Column case 'd': - return err.DataTypeName + return e.DataTypeName case 'n': - return err.Constraint + return e.Constraint case 'F': - return err.File + return e.File case 'L': - return err.Line + return e.Line case 'R': - return err.Routine + return e.Routine } return "" } -func (err *Error) Error() string { - return "pq: " + err.Message +func (e *Error) Error() string { + if e.Code != "" { + return "pq: " + e.Message + " (" + string(e.Code) + ")" + } + return "pq: " + e.Message } -// PGError is an interface used by previous versions of pq. It is provided -// only to support legacy code. New code should use the Error type. +// ErrorWithDetail returns the error message with detailed information and +// location context (if any). +// +// See the documentation on [Error]. +func (e *Error) ErrorWithDetail() string { + b := new(strings.Builder) + b.Grow(len(e.Message) + len(e.Detail) + len(e.Hint) + 30) + b.WriteString("ERROR: ") + b.WriteString(e.Message) + if e.Code != "" { + b.WriteString(" (") + b.WriteString(string(e.Code)) + b.WriteByte(')') + } + if e.Detail != "" { + b.WriteString("\nDETAIL: ") + b.WriteString(e.Detail) + } + if e.Hint != "" { + b.WriteString("\nHINT: ") + b.WriteString(e.Hint) + } + + if e.query != "" && e.Position != "" { + b.Grow(512) + pos, err := strconv.Atoi(e.Position) + if err != nil { + return b.String() + } + lines := strings.Split(e.query, "\n") + line, col := posToLine(pos, lines) + + fmt.Fprintf(b, "\nCONTEXT: line %d, column %d:\n\n", line, col) + if line > 2 { + fmt.Fprintf(b, "% 7d | %s\n", line-2, expandTab(lines[line-3])) + } + if line > 1 { + fmt.Fprintf(b, "% 7d | %s\n", line-1, expandTab(lines[line-2])) + } + /// Expand tabs, so that the ^ is at at the correct position, but leave + /// "column 10-13" intact. Adjusting this to the visual column would be + /// better, but we don't know the tabsize of the user in their editor, + /// which can be 8, 4, 2, or something else. We can't know. So leaving + /// it as the character index is probably the "most correct". + expanded := expandTab(lines[line-1]) + diff := len(expanded) - len(lines[line-1]) + fmt.Fprintf(b, "% 7d | %s\n", line, expanded) + fmt.Fprintf(b, "% 10s%s%s\n", "", strings.Repeat(" ", col-1+diff), "^") + } + + return b.String() +} + +// PGError is an interface used by previous versions of pq. +// +// Deprecated: use the Error type. This is never used. type PGError interface { Error() string Fatal() bool @@ -483,15 +618,18 @@ func errRecoverNoErrBadConn(err *error) { } } -func (cn *conn) errRecover(err *error) { - e := recover() - switch v := e.(type) { +func (cn *conn) errRecover(err *error, query ...string) { + r := recover() + switch v := r.(type) { case nil: // Do nothing case runtime.Error: cn.err.set(driver.ErrBadConn) panic(v) case *Error: + if len(query) > 0 && query[0] != "" { + v.query = query[0] + } if v.Fatal() { *err = driver.ErrBadConn } else { @@ -512,7 +650,7 @@ func (cn *conn) errRecover(err *error) { default: cn.err.set(driver.ErrBadConn) - panic(fmt.Sprintf("unknown error: %#v", e)) + panic(fmt.Sprintf("unknown error: %#v", r)) } // Any time we return ErrBadConn, we need to remember it since *Tx doesn't @@ -521,3 +659,47 @@ func (cn *conn) errRecover(err *error) { cn.err.set(driver.ErrBadConn) } } + +func posToLine(pos int, lines []string) (line, col int) { + read := 0 + for i := range lines { + line++ + ll := utf8.RuneCountInString(lines[i]) + 1 // +1 for the removed newline + if read+ll >= pos { + col = pos - read + if col < 1 { // Should never happen, but just in case. + col = 1 + } + break + } + read += ll + } + return line, col +} + +func expandTab(s string) string { + var ( + b strings.Builder + l int + fill = func(n int) string { + b := make([]byte, n) + for i := range b { + b[i] = ' ' + } + return string(b) + } + ) + b.Grow(len(s)) + for _, r := range s { + switch r { + case '\t': + tw := 8 - l%8 + b.WriteString(fill(tw)) + l += tw + default: + b.WriteRune(r) + l += 1 + } + } + return b.String() +} diff --git a/error_test.go b/error_test.go new file mode 100644 index 000000000..e2ac8fb0c --- /dev/null +++ b/error_test.go @@ -0,0 +1,142 @@ +package pq + +import ( + "errors" + "testing" + + "github.com/lib/pq/internal/pqtest" +) + +func TestErrorSQLState(t *testing.T) { + r := readBuf([]byte{67, 52, 48, 48, 48, 49, 0, 0}) // 40001 + err := parseError(&r, "") + var sqlErr interface{ SQLState() string } + if !errors.As(err, &sqlErr) { + t.Fatal("SQLState interface not satisfied") + } + if state := err.SQLState(); state != "40001" { + t.Fatalf("unexpected SQL state %v", state) + } +} + +func TestError(t *testing.T) { + tests := []struct { + in, want, wantDetail string + }{ + {`create schema pg_xx`, `pq: unacceptable schema name "pg_xx" (42939)`, ` + ERROR: unacceptable schema name "pg_xx" (42939) + DETAIL: The prefix "pg_" is reserved for system schemas. + `}, + {`create view x as select 1; copy x to stdout`, `pq: cannot copy from view "x" (42809)`, ` + ERROR: cannot copy from view "x" (42809) + HINT: Try the COPY (SELECT ...) TO variant. + `}, + {`select columndoesntexist`, `pq: column "columndoesntexist" does not exist (42703)`, ` + ERROR: column "columndoesntexist" does not exist (42703) + CONTEXT: line 1, column 8: + + 1 | select columndoesntexist + ^ + `}, + {`select !@#`, "pq: syntax error at end of input (42601)", ` + ERROR: syntax error at end of input (42601) + CONTEXT: line 1, column 11: + + 1 | select !@# + ^ + `}, + {"select 'asd',\n\t'asd'::jsonb", "pq: invalid input syntax for type json (22P02)", ` + ERROR: invalid input syntax for type json (22P02) + DETAIL: Token "asd" is invalid. + CONTEXT: line 2, column 2: + + 1 | select 'asd', + 2 | 'asd'::jsonb + ^ + `}, + {"select 'asd'\n,'zxc',\n'def',\n123,\n'foo', 'asd'::jsonb", "pq: invalid input syntax for type json (22P02)", ` + ERROR: invalid input syntax for type json (22P02) + DETAIL: Token "asd" is invalid. + CONTEXT: line 5, column 8: + + 3 | 'def', + 4 | 123, + 5 | 'foo', 'asd'::jsonb + ^ + `}, + {"select '€€€', a", `pq: column "a" does not exist (42703)`, ` + ERROR: column "a" does not exist (42703) + CONTEXT: line 1, column 15: + + 1 | select '€€€', a + ^ + `}, + {"select '€€€',\n'€',a", `pq: column "a" does not exist (42703)`, ` + ERROR: column "a" does not exist (42703) + CONTEXT: line 2, column 5: + + 1 | select '€€€', + 2 | '€',a + ^ + `}, + {pqtest.NormalizeIndent(` + create table browsers ( + browser_id serial, + name varchar, + version varchar + ); + create unique index "browsers#name#version" on browsers(name, version); + + create table systems ( + system_id serial, + name varchar, + version varchar, + ); + create unique index "systems#name#version" on systems(name, version); + `), `pq: syntax error at or near ")" (42601)`, ` + ERROR: syntax error at or near ")" (42601) + CONTEXT: line 12, column 1: + + 10 | name varchar, + 11 | version varchar, + 12 | ); + ^ + `}, + + {pqtest.NormalizeIndent(` + create table browsers (browser_id serial, name varchar, version varchar); create unique index "browsers#name#version" on browsers(name, version); + create table systems (system_id serial, name varchar, version varchar,); create unique index "systems#name#version" on systems(name, version); + `), `pq: syntax error at or near ")" (42601)`, ` + ERROR: syntax error at or near ")" (42601) + CONTEXT: line 2, column 71: + + 1 | create table browsers (browser_id serial, name varchar, version varchar); create unique index "browsers#name#version" on browsers(name, version); + 2 | create table systems (system_id serial, name varchar, version varchar,); create unique index "systems#name#version" on systems(name, version); + ^ + `}, + } + + db := pqtest.MustDB(t) + + for _, tt := range tests { + _, err := db.Exec(tt.in) + if err == nil { + t.Fatal("no error?") + } + pqErr := new(Error) + if !errors.As(err, &pqErr) { + t.Fatalf("wrong error %T: %[1]s", err) + } + + if err.Error() != tt.want { + t.Errorf("\nhave: %s\nwant: %s", err.Error(), tt.want) + } + tt.wantDetail = pqtest.NormalizeIndent(tt.wantDetail) + if pqErr.query != "" && pqErr.Position != "" { + tt.wantDetail += "\n" + } + if pqErr.ErrorWithDetail() != tt.wantDetail { + t.Errorf("\nhave:\n%s\nwant:\n%s", pqErr.ErrorWithDetail(), tt.wantDetail) + } + } +} diff --git a/go18_test.go b/go18_test.go index e058cfa39..1d70fe7f3 100644 --- a/go18_test.go +++ b/go18_test.go @@ -329,19 +329,3 @@ func TestTxOptions(t *testing.T) { t.Errorf("Expected error to mention isolation level, got %q", err) } } - -func TestErrorSQLState(t *testing.T) { - r := readBuf([]byte{67, 52, 48, 48, 48, 49, 0, 0}) // 40001 - err := parseError(&r) - var sqlErr errWithSQLState - if !errors.As(err, &sqlErr) { - t.Fatal("SQLState interface not satisfied") - } - if state := err.SQLState(); state != "40001" { - t.Fatalf("unexpected SQL state %v", state) - } -} - -type errWithSQLState interface { - SQLState() string -} diff --git a/internal/pqtest/pqtest.go b/internal/pqtest/pqtest.go index 1134e7e01..e72edcec8 100644 --- a/internal/pqtest/pqtest.go +++ b/internal/pqtest/pqtest.go @@ -76,18 +76,3 @@ func MustDB(t testing.TB) *sql.DB { } return conn } - -// ErrorContains checks if the error message in have contains the text in -// want. -// -// This is safe when have is nil. Use an empty string for want if you want to -// test that err is nil. -func ErrorContains(have error, want string) bool { - if have == nil { - return want == "" - } - if want == "" { - return false - } - return strings.Contains(have.Error(), want) -} diff --git a/internal/pqtest/ztest.go b/internal/pqtest/ztest.go new file mode 100644 index 000000000..825fd4e6c --- /dev/null +++ b/internal/pqtest/ztest.go @@ -0,0 +1,55 @@ +// Copied from https://github.com/arp242/zstd/tree/main/ztest + +package pqtest + +import "strings" + +// ErrorContains checks if the error message in have contains the text in +// want. +// +// This is safe when have is nil. Use an empty string for want if you want to +// test that err is nil. +func ErrorContains(have error, want string) bool { + if have == nil { + return want == "" + } + if want == "" { + return false + } + return strings.Contains(have.Error(), want) +} + +// NormalizeIndent removes tab indentation from every line. +// +// This is useful for "inline" multiline strings: +// +// cases := []struct { +// string in +// }{ +// ` +// Hello, +// world! +// `, +// } +// +// This is nice and readable, but the downside is that every line will now have +// two extra tabs. This will remove those two tabs from every line. +// +// The amount of tabs to remove is based only on the first line, any further +// tabs will be preserved. +func NormalizeIndent(in string) string { + indent := 0 + for _, c := range strings.TrimLeft(in, "\n") { + if c != '\t' { + break + } + indent++ + } + + r := "" + for _, line := range strings.Split(in, "\n") { + r += strings.Replace(line, "\t", "", indent) + "\n" + } + + return strings.TrimSpace(r) +} diff --git a/notify.go b/notify.go index fe0fc0e5a..0eafabc63 100644 --- a/notify.go +++ b/notify.go @@ -210,9 +210,9 @@ func (l *ListenerConn) listenerConnLoop() (err error) { // that, but we should make sure that the error we display is the // one from the stray ErrorResponse, not io.ErrUnexpectedEOF. if !l.setState(connStateExpectReadyForQuery) { - return parseError(r) + return parseError(r, "") } - l.replyChan <- message{t, parseError(r)} + l.replyChan <- message{t, parseError(r, "")} case 'C', 'I': if !l.setState(connStateExpectReadyForQuery) { @@ -232,7 +232,7 @@ func (l *ListenerConn) listenerConnLoop() (err error) { // ignore case 'N': if n := l.cn.noticeHandler; n != nil { - n(parseError(r)) + n(parseError(r, "")) } default: return fmt.Errorf("unexpected message %q from server in listenerConnLoop", t)