Skip to content

Commit

Permalink
sql: set constraint name in pg error for FK violations
Browse files Browse the repository at this point in the history
This change adds infrastructure to set the "constraint name" field of
errors on pgwire and uses it to populate the constraint name for
foreign key violation errors.

Fixes cockroachdb#36494.

Release note (sql change): foreign key violation errors now fill in
the "constraint name" error message field.
  • Loading branch information
RaduBerinde committed Sep 14, 2020
1 parent 38115d0 commit 7742c02
Show file tree
Hide file tree
Showing 14 changed files with 295 additions and 57 deletions.
6 changes: 5 additions & 1 deletion pkg/cli/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,13 @@ func (f *formattedError) Error() string {
severity := "ERROR"

// Extract the fields.
var message, hint, detail, location string
var message, hint, detail, location, constraintName string
var code pgcode.Code
if pqErr := (*pq.Error)(nil); errors.As(f.err, &pqErr) {
if pqErr.Severity != "" {
severity = pqErr.Severity
}
constraintName = pqErr.Constraint
message = pqErr.Message
code = pgcode.MakeCode(string(pqErr.Code))
hint, detail = pqErr.Hint, pqErr.Detail
Expand Down Expand Up @@ -118,6 +119,9 @@ func (f *formattedError) Error() string {
if detail != "" {
fmt.Fprintln(&buf, "DETAIL:", detail)
}
if constraintName != "" {
fmt.Fprintln(&buf, "CONSTRAINT:", constraintName)
}
if hint != "" {
fmt.Fprintln(&buf, "HINT:", hint)
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/sql/opt/exec/execbuilder/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -776,11 +776,13 @@ func mkFKCheckErr(md *opt.Metadata, c *memo.FKChecksItem, keyVals tree.Datums) e
referenced := md.TableMeta(c.ReferencedTable)

var msg, details bytes.Buffer
var constraintName string
if c.FKOutbound {
// Generate an error of the form:
// ERROR: insert on table "child" violates foreign key constraint "foo"
// DETAIL: Key (child_p)=(2) is not present in table "parent".
fk := origin.Table.OutboundForeignKey(c.FKOrdinal)
constraintName = fk.Name()
fmt.Fprintf(&msg, "%s on table ", c.OpName)
lex.EncodeEscapedSQLIdent(&msg, string(origin.Alias.ObjectName))
msg.WriteString(" violates foreign key constraint ")
Expand Down Expand Up @@ -822,6 +824,7 @@ func mkFKCheckErr(md *opt.Metadata, c *memo.FKChecksItem, keyVals tree.Datums) e
// "child_child_p_fkey" on table "child"
// DETAIL: Key (p)=(1) is still referenced from table "child".
fk := referenced.Table.InboundForeignKey(c.FKOrdinal)
constraintName = fk.Name()
fmt.Fprintf(&msg, "%s on table ", c.OpName)
lex.EncodeEscapedSQLIdent(&msg, string(referenced.Alias.ObjectName))
msg.WriteString(" violates foreign key constraint ")
Expand Down Expand Up @@ -850,7 +853,10 @@ func mkFKCheckErr(md *opt.Metadata, c *memo.FKChecksItem, keyVals tree.Datums) e
}

return errors.WithDetail(
pgerror.Newf(pgcode.ForeignKeyViolation, "%s", msg.String()),
pgerror.WithConstraintName(
pgerror.Newf(pgcode.ForeignKeyViolation, "%s", msg.String()),
constraintName,
),
details.String(),
)
}
Expand Down
9 changes: 7 additions & 2 deletions pkg/sql/pgwire/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -1252,15 +1252,20 @@ func writeErrFields(
msgBuilder.writeTerminatedString(pgErr.Code)

if pgErr.Detail != "" {
msgBuilder.putErrFieldMsg(pgwirebase.ServerErrFileldDetail)
msgBuilder.putErrFieldMsg(pgwirebase.ServerErrFieldDetail)
msgBuilder.writeTerminatedString(pgErr.Detail)
}

if pgErr.Hint != "" {
msgBuilder.putErrFieldMsg(pgwirebase.ServerErrFileldHint)
msgBuilder.putErrFieldMsg(pgwirebase.ServerErrFieldHint)
msgBuilder.writeTerminatedString(pgErr.Hint)
}

if pgErr.ConstraintName != "" {
msgBuilder.putErrFieldMsg(pgwirebase.ServerErrFieldConstraintName)
msgBuilder.writeTerminatedString(pgErr.ConstraintName)
}

if pgErr.Source != nil {
errCtx := pgErr.Source
if errCtx.File != "" {
Expand Down
85 changes: 85 additions & 0 deletions pkg/sql/pgwire/pgerror/constraint_name.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright 2020 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package pgerror

import (
"context"
"fmt"

"github.com/cockroachdb/errors"
"github.com/gogo/protobuf/proto"
)

// WithConstraintName decorates the error with a severity.
func WithConstraintName(err error, constraint string) error {
if err == nil {
return nil
}

return &withConstraintName{cause: err, constraint: constraint}
}

// GetConstraintName attempts to unwrap and find a Severity.
func GetConstraintName(err error) string {
if c := (*withConstraintName)(nil); errors.As(err, &c) {
return c.constraint
}
return ""
}

type withConstraintName struct {
cause error
constraint string
}

var _ error = (*withConstraintName)(nil)
var _ errors.SafeDetailer = (*withConstraintName)(nil)
var _ fmt.Formatter = (*withConstraintName)(nil)
var _ errors.SafeFormatter = (*withConstraintName)(nil)

func (w *withConstraintName) Error() string { return w.cause.Error() }
func (w *withConstraintName) Cause() error { return w.cause }
func (w *withConstraintName) Unwrap() error { return w.cause }
func (w *withConstraintName) SafeDetails() []string {
// The constraint name is considered PII.
return nil
}

func (w *withConstraintName) Format(s fmt.State, verb rune) { errors.FormatError(w, s, verb) }

func (w *withConstraintName) SafeFormatError(p errors.Printer) (next error) {
if p.Detail() {
p.Printf("constraint name: %s", w.constraint)
}
return w.cause
}

// decodeWithConstraintName is a custom decoder that will be used when decoding
// withConstraintName error objects.
// Note that as the last argument it takes proto.Message (and not
// protoutil.Message which is required by linter) because the latter brings in
// additional dependencies into this package and the former is sufficient here.
func decodeWithConstraintName(
_ context.Context, cause error, _ string, details []string, _ proto.Message,
) error {
var constraint string
if len(details) > 0 {
constraint = details[0]
}
return &withConstraintName{cause: cause, constraint: constraint}
}

func init() {
errors.RegisterWrapperDecoder(
errors.GetTypeKey((*withConstraintName)(nil)),
decodeWithConstraintName,
)
}
41 changes: 41 additions & 0 deletions pkg/sql/pgwire/pgerror/constraint_name_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2020 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package pgerror

import (
"fmt"
"testing"

"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/errors"
"github.com/stretchr/testify/require"
)

func TestConstraintName(t *testing.T) {
testCases := []struct {
err error
expectedConstraint string
}{
{WithConstraintName(fmt.Errorf("test"), "fk1"), "fk1"},
{WithConstraintName(WithConstraintName(fmt.Errorf("test"), "fk1"), "fk2"), "fk2"},
{WithConstraintName(WithCandidateCode(fmt.Errorf("test"), pgcode.FeatureNotSupported), "fk1"), "fk1"},
{New(pgcode.Uncategorized, "i am an error"), ""},
{WithCandidateCode(WithConstraintName(errors.Newf("test"), "fk1"), pgcode.System), "fk1"},
{fmt.Errorf("something else"), ""},
}

for _, tc := range testCases {
t.Run(tc.err.Error(), func(t *testing.T) {
constraint := GetConstraintName(tc.err)
require.Equal(t, tc.expectedConstraint, constraint)
})
}
}
100 changes: 71 additions & 29 deletions pkg/sql/pgwire/pgerror/errors.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/sql/pgwire/pgerror/errors.proto
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ message Error {
string detail = 3;
string hint = 4;
string severity = 8;
string constraint_name = 9;

message Source {
string file = 1;
Expand Down
7 changes: 4 additions & 3 deletions pkg/sql/pgwire/pgerror/flatten.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ func Flatten(err error) *Error {
return nil
}
resErr := &Error{
Code: GetPGCode(err).String(),
Message: err.Error(),
Severity: GetSeverity(err),
Code: GetPGCode(err).String(),
Message: err.Error(),
Severity: GetSeverity(err),
ConstraintName: GetConstraintName(err),
}

// Populate the source field if available.
Expand Down
Loading

0 comments on commit 7742c02

Please sign in to comment.