Merge #54313 #54542

54313: sql: set constraint name in pg error for FK violations r=RaduBerinde a=RaduBerinde

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 #36494.

Release note (sql change): foreign key violation errors now fill in
the "constraint name" error message field.

54542: sql: enable partial inverted indexes in randomized tests r=rytaft a=mgartner

Now that partial inverted indexes can be created, they can be generated
in randomized tests.

Release note: None

Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>

pkg/cli/error.go

@@ -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
@@ -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)
 	}

pkg/sql/opt/exec/execbuilder/mutation.go

@@ -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 ")
@@ -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 ")
@@ -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(),
 	)
 }

pkg/sql/pgwire/conn.go

@@ -1253,15 +1253,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 != "" {

pkg/sql/pgwire/pgerror/constraint_name.go

@@ -0,0 +1,94 @@
+// 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/cockroachdb/errors/errorspb"
+	"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
+}
+
+func encodeWithConstraintName(_ context.Context, err error) (string, []string, proto.Message) {
+	w := err.(*withConstraintName)
+	return "", nil, &errorspb.StringPayload{Msg: w.constraint}
+}
+
+// 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, _ []string, payload proto.Message,
+) error {
+	m, ok := payload.(*errorspb.StringPayload)
+	if !ok {
+		// If this ever happens, this means some version of the library
+		// (presumably future) changed the payload type, and we're
+		// receiving this here. In this case, give up and let
+		// DecodeError use the opaque type.
+		return nil
+	}
+	return &withConstraintName{cause: cause, constraint: m.Msg}
+}
+
+func init() {
+	key := errors.GetTypeKey((*withConstraintName)(nil))
+	errors.RegisterWrapperEncoder(key, encodeWithConstraintName)
+	errors.RegisterWrapperDecoder(key, decodeWithConstraintName)
+}

pkg/sql/pgwire/pgerror/constraint_name_test.go

@@ -0,0 +1,48 @@
+// 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"
+	"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"), ""},
+		{WithConstraintName(fmt.Errorf("test"), "fk\"⌂"), "fk\"⌂"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.err.Error(), func(t *testing.T) {
+			constraint := GetConstraintName(tc.err)
+			require.Equal(t, tc.expectedConstraint, constraint)
+			// Test that the constraint name survives an encode/decode cycle.
+			enc := errors.EncodeError(context.Background(), tc.err)
+			err2 := errors.DecodeError(context.Background(), enc)
+			constraint = GetConstraintName(err2)
+			require.Equal(t, tc.expectedConstraint, constraint)
+		})
+	}
+}

pkg/sql/pgwire/pgerror/errors.proto

@@ -24,6 +24,7 @@ message Error {
   string detail = 3;
   string hint = 4;
   string severity = 8;
+  string constraint_name = 9;
 
   message Source {
       string file = 1;

pkg/sql/pgwire/pgerror/flatten.go

@@ -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.