diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index c8f35c450ee3..fc0d8c8409a6 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -1844,7 +1844,7 @@ CREATE TABLE crdb_internal.zones ( for _, s := range subzones { index, err := table.FindIndexByID(sqlbase.IndexID(s.IndexID)) if err != nil { - if err == sqlbase.ErrIndexGCMutationsList { + if pgerror.IsMarkedError(err, sqlbase.ErrIndexGCMutationsList) { continue } return err diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go index 4638158743a7..2bc87b99a8f3 100644 --- a/pkg/sql/exec_util.go +++ b/pkg/sql/exec_util.go @@ -976,7 +976,7 @@ func (scc *schemaChangerCollection) execSchemaChanges( log.Warningf(ctx, "error executing schema change: %s", err) } - if err == sqlbase.ErrDescriptorNotFound || err == ctx.Err() { + if pgerror.IsMarkedError(err, sqlbase.ErrDescriptorNotFound) || err == ctx.Err() { // 1. If the descriptor is dropped while the schema change // is executing, the schema change is considered completed. // 2. If the context is canceled the schema changer quits here diff --git a/pkg/sql/lease.go b/pkg/sql/lease.go index 452006916aa1..114271691c4d 100644 --- a/pkg/sql/lease.go +++ b/pkg/sql/lease.go @@ -46,8 +46,8 @@ import ( "github.com/pkg/errors" ) -var errRenewLease = errors.New("renew lease on id") -var errReadOlderTableVersion = errors.New("read older table version from store") +var errRenewLease = pgerror.WithMarker(errors.New("renew lease on id")) +var errReadOlderTableVersion = pgerror.WithMarker(errors.New("read older table version from store")) // A lease stored in system.lease. type storedTableLease struct { @@ -337,7 +337,9 @@ func maybeIncrementVersion( return nil } -var errDidntUpdateDescriptor = errors.New("didn't update the table descriptor") +var errDidntUpdateDescriptor = pgerror.WithMarker(errors.New("didn't update the table descriptor")) + +var errLeaseVersionChanged = pgerror.WithMarker(errors.New("lease version changed")) // Publish updates a table descriptor. It also maintains the invariant that // there are at most two versions of the descriptor out in the wild at any time @@ -355,7 +357,6 @@ func (s LeaseStore) Publish( update func(*sqlbase.MutableTableDescriptor) error, logEvent func(*client.Txn) error, ) (*sqlbase.ImmutableTableDescriptor, error) { - errLeaseVersionChanged := errors.New("lease version changed") // Retry while getting errLeaseVersionChanged. for r := retry.Start(base.DefaultRetryOptions()); r.Next(); { // Wait until there are no unexpired leases on the previous version @@ -428,10 +429,10 @@ func (s LeaseStore) Publish( return txn.CommitInBatch(ctx, b) }) - switch err { - case nil, errDidntUpdateDescriptor: + switch { + case err == nil, pgerror.IsMarkedError(err, errDidntUpdateDescriptor): return sqlbase.NewImmutableTableDescriptor(tableDesc.TableDescriptor), nil - case errLeaseVersionChanged: + case pgerror.IsMarkedError(err, errLeaseVersionChanged): // will loop around to retry default: return nil, err @@ -1061,7 +1062,7 @@ func purgeOldVersions( // active lease, so that it doesn't get released when removeInactives() // is called below. Release this lease after calling removeInactives(). table, _, err := t.findForTimestamp(ctx, m.execCfg.Clock.Now()) - if dropped := err == errTableDropped; dropped || err == nil { + if dropped := pgerror.IsMarkedError(err, errTableDropped); dropped || err == nil { removeInactives(dropped) if table != nil { s, err := t.release(&table.ImmutableTableDescriptor, m.removeOnceDereferenced()) @@ -1531,8 +1532,9 @@ func (m *LeaseManager) Acquire( } return &table.ImmutableTableDescriptor, table.expiration, nil } - switch err { - case errRenewLease: + + switch { + case pgerror.IsMarkedError(err, errRenewLease): // Renew lease and retry. This will block until the lease is acquired. if _, errLease := acquireNodeLease(ctx, m, tableID); errLease != nil { return nil, hlc.Timestamp{}, errLease @@ -1541,7 +1543,7 @@ func (m *LeaseManager) Acquire( m.testingKnobs.LeaseStoreTestingKnobs.LeaseAcquireResultBlockEvent(LeaseAcquireBlock) } - case errReadOlderTableVersion: + case pgerror.IsMarkedError(err, errReadOlderTableVersion): // Read old table versions from the store. This can block while reading // old table versions from the store. versions, errRead := m.readOlderVersionForTimestamp(ctx, tableID, timestamp) diff --git a/pkg/sql/opt_catalog.go b/pkg/sql/opt_catalog.go index 1eff0581417b..d0054e0de995 100644 --- a/pkg/sql/opt_catalog.go +++ b/pkg/sql/opt_catalog.go @@ -155,7 +155,7 @@ func (oc *optCatalog) ResolveDataSourceByID( tableLookup, err := oc.planner.LookupTableByID(ctx, sqlbase.ID(dataSourceID)) if err != nil || tableLookup.IsAdding { - if err == sqlbase.ErrDescriptorNotFound || tableLookup.IsAdding { + if pgerror.IsMarkedError(err, sqlbase.ErrDescriptorNotFound) || tableLookup.IsAdding { return nil, sqlbase.NewUndefinedRelationError(&tree.TableRef{TableID: int64(dataSourceID)}) } return nil, err diff --git a/pkg/sql/pgwire/pgerror/errors.go b/pkg/sql/pgwire/pgerror/errors.go index 04cfa27d439a..0267e856ab09 100644 --- a/pkg/sql/pgwire/pgerror/errors.go +++ b/pkg/sql/pgwire/pgerror/errors.go @@ -64,11 +64,13 @@ func formatMsgHintDetail(prefix, msg, hint, detail string) string { // information at the specified depth level. func NewErrorWithDepthf(depth int, code string, format string, args ...interface{}) *Error { srcCtx := makeSrcCtx(depth + 1) - return &Error{ + err := &Error{ Message: fmt.Sprintf(format, args...), Code: code, Source: &srcCtx, } + err.OriginMarker = getMarker(err) + return err } // NewError creates an Error. @@ -221,7 +223,7 @@ var _ fmt.Formatter = &Error{} // Format implements the fmt.Formatter interface. // -// %v/%s prints the rror as usual. +// %v/%s prints the error as usual. // %#v adds the pg error code at the beginning. // %+v prints all the details, including the embedded stack traces. func (pg *Error) Format(s fmt.State, verb rune) { diff --git a/pkg/sql/pgwire/pgerror/errors.pb.go b/pkg/sql/pgwire/pgerror/errors.pb.go index 2c6e7d49c3d8..0b63fcf5abc7 100644 --- a/pkg/sql/pgwire/pgerror/errors.pb.go +++ b/pkg/sql/pgwire/pgerror/errors.pb.go @@ -37,16 +37,21 @@ type Error struct { TelemetryKey string `protobuf:"bytes,6,opt,name=telemetry_key,json=telemetryKey,proto3" json:"telemetry_key,omitempty"` // complement to the detail field that can be reported // in sentry reports. This is scrubbed of PII. - SafeDetail []*Error_SafeDetail `protobuf:"bytes,7,rep,name=safe_detail,json=safeDetail,proto3" json:"safe_detail,omitempty"` - XXX_NoUnkeyedLiteral struct{} `json:"-"` - XXX_sizecache int32 `json:"-"` + SafeDetail []*Error_SafeDetail `protobuf:"bytes,7,rep,name=safe_detail,json=safeDetail,proto3" json:"safe_detail,omitempty"` + // the origin marker is guaranteed to propagate as-is + // through error wraps, and can be used to recognize + // the original cause of a pgerror. + // This may contain PII and should not be reported. + OriginMarker string `protobuf:"bytes,8,opt,name=origin_marker,json=originMarker,proto3" json:"origin_marker,omitempty"` + XXX_NoUnkeyedLiteral struct{} `json:"-"` + XXX_sizecache int32 `json:"-"` } func (m *Error) Reset() { *m = Error{} } func (m *Error) String() string { return proto.CompactTextString(m) } func (*Error) ProtoMessage() {} func (*Error) Descriptor() ([]byte, []int) { - return fileDescriptor_errors_a71973a7ba445e50, []int{0} + return fileDescriptor_errors_7e07537ae42edeaa, []int{0} } func (m *Error) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -83,7 +88,7 @@ func (m *Error_Source) Reset() { *m = Error_Source{} } func (m *Error_Source) String() string { return proto.CompactTextString(m) } func (*Error_Source) ProtoMessage() {} func (*Error_Source) Descriptor() ([]byte, []int) { - return fileDescriptor_errors_a71973a7ba445e50, []int{0, 0} + return fileDescriptor_errors_7e07537ae42edeaa, []int{0, 0} } func (m *Error_Source) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -119,7 +124,7 @@ func (m *Error_SafeDetail) Reset() { *m = Error_SafeDetail{} } func (m *Error_SafeDetail) String() string { return proto.CompactTextString(m) } func (*Error_SafeDetail) ProtoMessage() {} func (*Error_SafeDetail) Descriptor() ([]byte, []int) { - return fileDescriptor_errors_a71973a7ba445e50, []int{0, 1} + return fileDescriptor_errors_7e07537ae42edeaa, []int{0, 1} } func (m *Error_SafeDetail) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -216,6 +221,12 @@ func (m *Error) MarshalTo(dAtA []byte) (int, error) { i += n } } + if len(m.OriginMarker) > 0 { + dAtA[i] = 0x42 + i++ + i = encodeVarintErrors(dAtA, i, uint64(len(m.OriginMarker))) + i += copy(dAtA[i:], m.OriginMarker) + } return i, nil } @@ -329,6 +340,10 @@ func (m *Error) Size() (n int) { n += 1 + l + sovErrors(uint64(l)) } } + l = len(m.OriginMarker) + if l > 0 { + n += 1 + l + sovErrors(uint64(l)) + } return n } @@ -620,6 +635,35 @@ func (m *Error) Unmarshal(dAtA []byte) error { return err } iNdEx = postIndex + case 8: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field OriginMarker", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowErrors + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= (uint64(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthErrors + } + postIndex := iNdEx + intStringLen + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.OriginMarker = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex default: iNdEx = preIndex skippy, err := skipErrors(dAtA[iNdEx:]) @@ -982,31 +1026,32 @@ var ( ) func init() { - proto.RegisterFile("sql/pgwire/pgerror/errors.proto", fileDescriptor_errors_a71973a7ba445e50) + proto.RegisterFile("sql/pgwire/pgerror/errors.proto", fileDescriptor_errors_7e07537ae42edeaa) } -var fileDescriptor_errors_a71973a7ba445e50 = []byte{ - // 346 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x74, 0x51, 0xb1, 0x4e, 0x2a, 0x41, - 0x14, 0x65, 0x1f, 0xb0, 0xbc, 0x77, 0xe1, 0x15, 0x8c, 0x89, 0x99, 0x50, 0x2c, 0x20, 0x0d, 0xd5, - 0x92, 0x60, 0x61, 0x6f, 0xb0, 0x52, 0x9b, 0xc5, 0xca, 0x66, 0x33, 0x0e, 0x77, 0x61, 0xc3, 0xb2, - 0x83, 0x33, 0x43, 0x0c, 0x7f, 0xe1, 0xc7, 0xf8, 0x11, 0x94, 0x96, 0x96, 0xba, 0xfe, 0x88, 0x99, - 0xd9, 0x61, 0x2d, 0x8c, 0xcd, 0xe4, 0xdc, 0x7b, 0x6e, 0xce, 0x3d, 0xf7, 0x0c, 0xf4, 0xd5, 0x63, - 0x36, 0xd9, 0x2e, 0x9f, 0x52, 0x89, 0x93, 0xed, 0x12, 0xa5, 0x14, 0x72, 0x62, 0x5f, 0x15, 0x6e, - 0xa5, 0xd0, 0x82, 0x74, 0xb9, 0xe0, 0x6b, 0x29, 0x18, 0x5f, 0x85, 0x8e, 0x3f, 0x7b, 0xa9, 0x43, - 0xf3, 0xca, 0x20, 0x42, 0xa0, 0xc1, 0xc5, 0x02, 0xa9, 0x37, 0xf0, 0xc6, 0xff, 0x22, 0x8b, 0x09, - 0x85, 0xd6, 0x06, 0x95, 0x62, 0x4b, 0xa4, 0x7f, 0x6c, 0xfb, 0x58, 0x92, 0x53, 0xf0, 0x17, 0xa8, - 0x59, 0x9a, 0xd1, 0xba, 0x25, 0x5c, 0x65, 0x54, 0x56, 0x69, 0xae, 0x69, 0xa3, 0x54, 0x31, 0x98, - 0x5c, 0x80, 0xaf, 0xc4, 0x4e, 0x72, 0xa4, 0xcd, 0x81, 0x37, 0x6e, 0x4f, 0xfb, 0xe1, 0x0f, 0x1f, - 0xa1, 0xf5, 0x10, 0xce, 0xed, 0x58, 0xe4, 0xc6, 0xc9, 0x08, 0xfe, 0x6b, 0xcc, 0x70, 0x83, 0x5a, - 0xee, 0xe3, 0x35, 0xee, 0xa9, 0x6f, 0x55, 0x3b, 0x55, 0xf3, 0x1a, 0xf7, 0x64, 0x06, 0x6d, 0xc5, - 0x12, 0x8c, 0x9d, 0x9d, 0xd6, 0xa0, 0x3e, 0x6e, 0x4f, 0x47, 0xbf, 0xaf, 0x60, 0x09, 0xce, 0xec, - 0x68, 0x04, 0xaa, 0xc2, 0xbd, 0x1b, 0xf0, 0xcb, 0xe5, 0xe6, 0x82, 0x24, 0xcd, 0xaa, 0x1c, 0x0c, - 0x36, 0xbd, 0x2c, 0xcd, 0xcb, 0x10, 0x9a, 0x91, 0xc5, 0xa4, 0x07, 0x7f, 0x93, 0x5d, 0xce, 0x75, - 0x2a, 0x72, 0x97, 0x41, 0x55, 0xf7, 0x62, 0x80, 0xef, 0x3d, 0x64, 0x08, 0x1d, 0xeb, 0xf0, 0x18, - 0x65, 0xa9, 0x6c, 0x5d, 0xdf, 0xba, 0x38, 0x43, 0x38, 0xc1, 0xdc, 0x44, 0xbe, 0x88, 0x95, 0x66, - 0x7c, 0x1d, 0x6b, 0xc9, 0xf8, 0x31, 0xf4, 0xae, 0xa3, 0xe6, 0x86, 0xb9, 0x33, 0xc4, 0xe5, 0xf0, - 0xf0, 0x11, 0xd4, 0x0e, 0x45, 0xe0, 0xbd, 0x16, 0x81, 0xf7, 0x56, 0x04, 0xde, 0x7b, 0x11, 0x78, - 0xcf, 0x9f, 0x41, 0xed, 0xbe, 0xe5, 0xce, 0x7d, 0xf0, 0xed, 0x9f, 0x9f, 0x7f, 0x05, 0x00, 0x00, - 0xff, 0xff, 0xec, 0x81, 0x3a, 0x52, 0x16, 0x02, 0x00, 0x00, +var fileDescriptor_errors_7e07537ae42edeaa = []byte{ + // 365 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x74, 0x52, 0x31, 0x6f, 0xe2, 0x30, + 0x14, 0x26, 0x07, 0x04, 0xce, 0x70, 0x03, 0x3e, 0xe9, 0x64, 0x31, 0x04, 0x38, 0x16, 0xa6, 0x20, + 0x71, 0xc3, 0xed, 0x15, 0x9d, 0x5a, 0x96, 0xd0, 0xa9, 0x4b, 0xe4, 0x9a, 0x97, 0x60, 0x25, 0xc4, + 0xd4, 0x36, 0xaa, 0xf8, 0x17, 0xfd, 0x59, 0x8c, 0x1d, 0xdb, 0xad, 0x4d, 0xff, 0x48, 0x65, 0xc7, + 0xa4, 0x43, 0xd5, 0x25, 0xfa, 0xde, 0xf7, 0x3d, 0xbd, 0xf7, 0xbd, 0xcf, 0x41, 0x23, 0x75, 0x9f, + 0xcf, 0xf7, 0xe9, 0x03, 0x97, 0x30, 0xdf, 0xa7, 0x20, 0xa5, 0x90, 0x73, 0xfb, 0x55, 0xe1, 0x5e, + 0x0a, 0x2d, 0xf0, 0x80, 0x09, 0x96, 0x49, 0x41, 0xd9, 0x36, 0x74, 0xfa, 0xdf, 0x97, 0x26, 0x6a, + 0x5f, 0x1a, 0x84, 0x31, 0x6a, 0x31, 0xb1, 0x01, 0xe2, 0x8d, 0xbd, 0xd9, 0xcf, 0xc8, 0x62, 0x4c, + 0x50, 0x67, 0x07, 0x4a, 0xd1, 0x14, 0xc8, 0x0f, 0x4b, 0x9f, 0x4b, 0xfc, 0x07, 0xf9, 0x1b, 0xd0, + 0x94, 0xe7, 0xa4, 0x69, 0x05, 0x57, 0x99, 0x29, 0x5b, 0x5e, 0x68, 0xd2, 0xaa, 0xa6, 0x18, 0x8c, + 0xff, 0x23, 0x5f, 0x89, 0x83, 0x64, 0x40, 0xda, 0x63, 0x6f, 0xd6, 0x5b, 0x8c, 0xc2, 0x2f, 0x3e, + 0x42, 0xeb, 0x21, 0x5c, 0xdb, 0xb6, 0xc8, 0xb5, 0xe3, 0x29, 0xfa, 0xa5, 0x21, 0x87, 0x1d, 0x68, + 0x79, 0x8c, 0x33, 0x38, 0x12, 0xdf, 0x4e, 0xed, 0xd7, 0xe4, 0x15, 0x1c, 0xf1, 0x12, 0xf5, 0x14, + 0x4d, 0x20, 0x76, 0x76, 0x3a, 0xe3, 0xe6, 0xac, 0xb7, 0x98, 0x7e, 0xbf, 0x82, 0x26, 0xb0, 0xb4, + 0xad, 0x11, 0x52, 0x35, 0x36, 0xab, 0x84, 0xe4, 0x29, 0x2f, 0xe2, 0x1d, 0x95, 0x19, 0x48, 0xd2, + 0xad, 0x56, 0x55, 0xe4, 0xca, 0x72, 0xc3, 0x6b, 0xe4, 0x57, 0x0e, 0xcd, 0x99, 0x09, 0xcf, 0xeb, + 0xb0, 0x0c, 0x36, 0x5c, 0xce, 0x8b, 0x2a, 0xa9, 0x76, 0x64, 0x31, 0x1e, 0xa2, 0x6e, 0x72, 0x28, + 0x98, 0xe6, 0xa2, 0x70, 0x41, 0xd5, 0xf5, 0x30, 0x46, 0xe8, 0xd3, 0x0c, 0x9e, 0xa0, 0xbe, 0x3d, + 0xe3, 0x9c, 0x77, 0x35, 0xd9, 0x9e, 0xb6, 0x72, 0x99, 0x87, 0xe8, 0x37, 0x14, 0xe6, 0x5d, 0x36, + 0xb1, 0xd2, 0x94, 0x65, 0xb1, 0x96, 0x94, 0x9d, 0x5f, 0x66, 0xe0, 0xa4, 0xb5, 0x51, 0x6e, 0x8c, + 0x70, 0x31, 0x39, 0xbd, 0x05, 0x8d, 0x53, 0x19, 0x78, 0x4f, 0x65, 0xe0, 0x3d, 0x97, 0x81, 0xf7, + 0x5a, 0x06, 0xde, 0xe3, 0x7b, 0xd0, 0xb8, 0xed, 0xb8, 0x4c, 0xee, 0x7c, 0xfb, 0x63, 0xfc, 0xfb, + 0x08, 0x00, 0x00, 0xff, 0xff, 0xa0, 0xad, 0x99, 0x81, 0x3b, 0x02, 0x00, 0x00, } diff --git a/pkg/sql/pgwire/pgerror/errors.proto b/pkg/sql/pgwire/pgerror/errors.proto index 95f18f73e700..d00194babd76 100644 --- a/pkg/sql/pgwire/pgerror/errors.proto +++ b/pkg/sql/pgwire/pgerror/errors.proto @@ -50,4 +50,10 @@ message Error { // complement to the detail field that can be reported // in sentry reports. This is scrubbed of PII. repeated SafeDetail safe_detail = 7; + + // the origin marker is guaranteed to propagate as-is + // through error wraps, and can be used to recognize + // the original cause of a pgerror. + // This may contain PII and should not be reported. + string origin_marker = 8; }; diff --git a/pkg/sql/pgwire/pgerror/markers.go b/pkg/sql/pgwire/pgerror/markers.go new file mode 100644 index 000000000000..748752847fa5 --- /dev/null +++ b/pkg/sql/pgwire/pgerror/markers.go @@ -0,0 +1,202 @@ +// Copyright 2019 The Cockroach Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +// implied. See the License for the specific language governing +// permissions and limitations under the License. + +package pgerror + +import ( + "context" + "fmt" + "io" + "reflect" + + "github.com/pkg/errors" +) + +// Marker errors are designed to solve a problem: recognize +// when a pgerror is derived from a particular error object. +// +// This can be used e.g. to recognized particular origins for errors. +// +// The rules are as follows: +// +// - undecorated errors are considered to have equivalent markers if +// they have the same type (including package path for the type), +// and for Error objects also if they have the same code. +// +// - use WithMarker(err) to decorate an error with a new marker that's +// derived from its original marker but includes the file path of +// the caller of WithMarker(). This can be used to produce separate +// markers from that of undecorated errors, for example to separate +// errors generated in different places with errors.New() using the +// same message. +// +// - if an error is already marked, WithMarker() will panic. +// +// - Wrap() in this package, and errors.Wrap(), preserve the marker. +// +// - use pgerror.IsMarkedError(err1, err2) to test whether two +// errors have equivalent markers. +// +// We use a string marker instead of the more common/traditional +// chaining through errors.Wrap() and the Cause() interface, +// because pgerror.Error objects must flow through the network +// via protobuf, and the errors' package Cause() mechanism +// is not protobuf-encodable. +// +// We use the file path specifically in WithMarker(): +// +// - instead of the package path, so that different error objects in +// the same package have different markers; +// +// - instead of including the line number or function name, so that +// the marker persists across minor code refactorings; this ensures +// comparability across mixed version clusters. + +// WithMarker decorates an error with its own string marker. +// The marker is derived from the original marker to also include +// the file path of the caller of WithMarker. +func WithMarker(err error) error { + // Generate a marker. + marker := getMarker(err) + + // Decorate the marker with the caller information. + src := makeSrcCtx(1) + decoratedMarker := fmt.Sprintf("%s\n%s", marker, src.File) + + // Get the decorated pg error or build a fresh one. + newErr := WrapWithDepthf(1, err, CodeUncategorizedError, "") + // We're guaranteed an *Error object when the input is not nil. + pgErr := newErr.(*Error) + + // If there's a mark already set and it's not the one we're adding, + // refuse to create a new one. + if pgErr.OriginMarker != "" && pgErr.OriginMarker != marker { + panic(fmt.Sprintf("error already marked as %q", pgErr.OriginMarker)) + } + + // Override the original marker with the new one. + pgErr.OriginMarker = decoratedMarker + return pgErr +} + +// WithSameMarker decorated an error with the same marker as another +// error. +func WithSameMarker(err error, refErr error) error { + if err == nil || refErr == nil { + panic("cannot mark nil error or using nil reference error") + } + + var marker string + pgRefErr, ok := GetPGCause(refErr) + if ok && pgRefErr.OriginMarker != "" { + marker = pgRefErr.OriginMarker + } else { + marker = getMarker(refErr) + } + + // Get the decorated pg error or build a fresh one. + newErr := WrapWithDepthf(1, err, CodeUncategorizedError, "") + // We're guaranteed an *Error object when the input is not nil. + pgErr := newErr.(*Error) + + // Assign the marker. + pgErr.OriginMarker = marker + return pgErr +} + +// IsMarkedError tests whether a provided error has the same mark as +// the error in the second argument. +// The boolean return is the yes/no answer. +// If the errors are the same object (same reference), the result is +// true. +// If the errors are both derived from an Error object +// and they have the same, non-empty OriginMarker, the +// function returns true. +// Otherwise, it returns false. +func IsMarkedError(errToTest error, markedErr error) bool { + if errToTest == markedErr { + return true + } + if errToTest == nil || markedErr == nil { + return false + } + + lMarker := getMarker(errToTest) + rMarker := getMarker(markedErr) + + return lMarker != "" && lMarker == rMarker +} + +// getMarker returns a portable identification for the error. We +// assume that an error is identified by the full type identification +// (including package path) of the error object and its full error +// message. +func getMarker(err error) string { + err = errors.Cause(err) + + pgErr, isPgError := err.(*Error) + if isPgError && pgErr.OriginMarker != "" { + return pgErr.OriginMarker + } + + prefix := err.Error() + if isPgError { + prefix = fmt.Sprintf("(%s) %s", pgErr.Code, prefix) + } + + // We'll need the type to get the type identification below. + t := reflect.TypeOf(err) + + // We want the path of the type "inside". + origT := t + for { + switch origT.Kind() { + case reflect.Array, reflect.Chan, reflect.Map, reflect.Ptr, reflect.Slice: + origT = origT.Elem() + continue + } + break + } + + // The "natural" representation of an error would put the package + // name first. However, because we're interested in comparing these + // markers and we want to reject different errors quickly, we want + // to put the fields with the highest variability earlier in the + // marker. Hence message first, then type, then package. + return fmt.Sprintf("%s\n%s\n%s", + prefix, + t.String(), + origT.PkgPath()) +} + +// ContextCanceledMarkerError can be used as parameter to +// IsMarkedError to recognize a pgerror that wrapped a +// context.Canceled error. +// This is provided for convenience only, to avoid recomputing +// the marker on every call to IsMarkerError(). +var ContextCanceledMarkerError = Wrap(context.Canceled, "", "") + +// ContextDeadlineExceededMarkerError can be used as parameter to +// IsMarkedError to recognize a pgerror that wrapped a +// context.DeadlineExceeded error. +// This is provided for convenience only, to avoid recomputing +// the marker on every call to IsMarkerError(). +var ContextDeadlineExceededMarkerError = Wrap(context.DeadlineExceeded, "", "") + +// IOEOFMarkerError can be used as parameter to +// IsMarkedError to recognize a pgerror that wrapped a +// io.EOF error. +// This is provided for convenience only, to avoid recomputing +// the marker on every call to IsMarkerError(). +var IOEOFMarkerError = Wrap(io.EOF, "", "") diff --git a/pkg/sql/pgwire/pgerror/markers_test.go b/pkg/sql/pgwire/pgerror/markers_test.go new file mode 100644 index 000000000000..66fcc3719580 --- /dev/null +++ b/pkg/sql/pgwire/pgerror/markers_test.go @@ -0,0 +1,70 @@ +// Copyright 2019 The Cockroach Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +// implied. See the License for the specific language governing +// permissions and limitations under the License. + +package pgerror + +import ( + "context" + e1 "errors" + "io" + "regexp" + "testing" + + e2 "github.com/pkg/errors" +) + +func TestMarkerEquivalence(t *testing.T) { + testData := []struct { + expEq bool + err1 error + err2 error + expectedRe *regexp.Regexp + }{ + {true, NewError(CodeSyntaxError, "woo"), NewError(CodeSyntaxError, "woo"), regexp.MustCompile(`woo\n.*pgerror.Error`)}, + {false, NewError(CodeSyntaxError, "woo"), NewError(CodeSystemError, "woo"), nil}, + {true, Wrapf(NewError(CodeSyntaxError, "woo"), CodeSystemError, "waa"), NewError(CodeSyntaxError, "woo"), nil}, + {true, e2.Wrap(NewError(CodeSyntaxError, "woo"), "waa"), NewError(CodeSyntaxError, "woo"), nil}, + {true, e2.WithMessage(e2.New("woo"), "waa"), e2.New("woo"), nil}, + {false, e1.New("woo"), e2.New("woo"), nil}, + + {true, io.EOF, io.EOF, regexp.MustCompile(`EOF\n.*errorString`)}, + {true, io.EOF, IOEOFMarkerError, nil}, + {false, io.EOF, WithMarker(io.EOF), nil}, + {true, WithMarker(io.EOF), WithMarker(io.EOF), regexp.MustCompile(`EOF\n.*\n.*\n.*/markers_test.go`)}, + + {true, context.Canceled, context.Canceled, regexp.MustCompile(`context canceled\n.*errorString`)}, + {true, context.Canceled, ContextCanceledMarkerError, nil}, + {false, context.Canceled, WithMarker(context.Canceled), nil}, + + {true, WithSameMarker(context.Canceled, ContextCanceledMarkerError), ContextCanceledMarkerError, nil}, + {true, WithSameMarker(context.Canceled, WithMarker(context.Canceled)), WithMarker(context.Canceled), nil}, + {true, WithSameMarker(e2.New("woo"), context.Canceled), context.Canceled, nil}, + {false, WithSameMarker(e2.New("woo"), context.Canceled), WithMarker(context.Canceled), nil}, + {false, WithSameMarker(e2.New("woo"), context.Canceled), e2.New("woo"), nil}, + } + + for i, test := range testData { + m1 := getMarker(test.err1) + m2 := getMarker(test.err2) + if test.expEq && m1 != m2 { + t.Errorf("%d: unexpected marker difference:\n%s\n-- vs. --\n%s", i, m1, m2) + } else if !test.expEq && m1 == m2 { + t.Errorf("%d: unexpected marker equivalence:\n%s", i, m1) + } + + if test.expectedRe != nil && !test.expectedRe.MatchString(m1) { + t.Errorf("%d: marker does not match %q:\n%s", i, test.expectedRe, m1) + } + } +} diff --git a/pkg/sql/pgwire/pgerror/wrap.go b/pkg/sql/pgwire/pgerror/wrap.go index cf1b47a84235..602cae3417e5 100644 --- a/pkg/sql/pgwire/pgerror/wrap.go +++ b/pkg/sql/pgwire/pgerror/wrap.go @@ -128,9 +128,11 @@ func collectErrForWrap(err error, fallbackCode string) *Error { pgErr = collectErrForWrap(cause.Cause(), fallbackCode) } else { // There's no discernible cause. Build a new pgerr from scratch. - pgErr = &Error{Code: fallbackCode} - // - // For certain types of errors, we'll override the fall back code, + pgErr = &Error{ + Code: fallbackCode, + OriginMarker: getMarker(err), + } + // to ensure the error flows back up properly. // We also need to re-define the interfaces here explicitly // because we can't use `roachpb` directly (import loop) diff --git a/pkg/sql/physical_schema_accessors.go b/pkg/sql/physical_schema_accessors.go index d95e1f3ba857..aea5b7030d43 100644 --- a/pkg/sql/physical_schema_accessors.go +++ b/pkg/sql/physical_schema_accessors.go @@ -19,6 +19,7 @@ import ( "context" "github.com/cockroachdb/cockroach/pkg/internal/client" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/util/encoding" @@ -168,7 +169,7 @@ func (a UncachedPhysicalAccessor) GetObjectDesc( // We have a descriptor. Is it in the right state? We'll keep it if // it is in the ADD state. - if err := filterTableState(desc); err == nil || err == errTableAdding { + if err := filterTableState(desc); err == nil || pgerror.IsMarkedError(err, errTableAdding) { // Immediately after a RENAME an old name still points to the // descriptor during the drain phase for the name. Do not // return a descriptor during draining. diff --git a/pkg/sql/planner.go b/pkg/sql/planner.go index b67e994c8bb4..71a3d4153f21 100644 --- a/pkg/sql/planner.go +++ b/pkg/sql/planner.go @@ -25,6 +25,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/server/serverpb" "github.com/cockroachdb/cockroach/pkg/sql/coltypes" "github.com/cockroachdb/cockroach/pkg/sql/parser" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/querycache" "github.com/cockroachdb/cockroach/pkg/sql/row" "github.com/cockroachdb/cockroach/pkg/sql/rowcontainer" @@ -401,7 +402,7 @@ func (p *planner) LookupTableByID(ctx context.Context, tableID sqlbase.ID) (row. flags := ObjectLookupFlags{CommonLookupFlags: CommonLookupFlags{avoidCached: p.avoidCachedDescriptors}} table, err := p.Tables().getTableVersionByID(ctx, p.txn, tableID, flags) if err != nil { - if err == errTableAdding { + if pgerror.IsMarkedError(err, errTableAdding) { return row.TableEntry{IsAdding: true}, nil } return row.TableEntry{}, err diff --git a/pkg/sql/row/fk_existence_base.go b/pkg/sql/row/fk_existence_base.go index d66d076cbc30..27f6e9715b97 100644 --- a/pkg/sql/row/fk_existence_base.go +++ b/pkg/sql/row/fk_existence_base.go @@ -236,4 +236,4 @@ func computeFkCheckColumnIDs( } } -var errSkipUnusedFK = errors.New("no columns involved in FK included in writer") +var errSkipUnusedFK = pgerror.WithMarker(errors.New("no columns involved in FK included in writer")) diff --git a/pkg/sql/row/fk_existence_delete.go b/pkg/sql/row/fk_existence_delete.go index 9be6865926a0..18e48d80743e 100644 --- a/pkg/sql/row/fk_existence_delete.go +++ b/pkg/sql/row/fk_existence_delete.go @@ -19,6 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/internal/client" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" ) @@ -63,7 +64,7 @@ func makeFkExistenceCheckHelperForDelete( continue } fk, err := makeFkExistenceCheckBaseHelper(txn, otherTables, idx, ref, colMap, alloc, CheckDeletes) - if err == errSkipUnusedFK { + if pgerror.IsMarkedError(err, errSkipUnusedFK) { continue } if err != nil { diff --git a/pkg/sql/row/fk_existence_insert.go b/pkg/sql/row/fk_existence_insert.go index 84bbf5d5f40d..c621e405e8fe 100644 --- a/pkg/sql/row/fk_existence_insert.go +++ b/pkg/sql/row/fk_existence_insert.go @@ -19,6 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/internal/client" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" ) @@ -67,7 +68,7 @@ func makeFkExistenceCheckHelperForInsert( for _, idx := range table.AllNonDropIndexes() { if idx.ForeignKey.IsSet() { fk, err := makeFkExistenceCheckBaseHelper(txn, otherTables, idx, idx.ForeignKey, colMap, alloc, CheckInserts) - if err == errSkipUnusedFK { + if pgerror.IsMarkedError(err, errSkipUnusedFK) { continue } if err != nil { diff --git a/pkg/sql/schema_changer.go b/pkg/sql/schema_changer.go index a3ba6d8bc3d0..fc7fc2124709 100644 --- a/pkg/sql/schema_changer.go +++ b/pkg/sql/schema_changer.go @@ -172,29 +172,24 @@ func isPermanentSchemaChangeError(err error) bool { return false } - switch err { + switch { case - context.Canceled, - context.DeadlineExceeded, - errExistingSchemaChangeLease, - errExpiredSchemaChangeLease, - errNotHitGCTTLDeadline, - errSchemaChangeDuringDrain, - errSchemaChangeNotFirstInLine: + pgerror.IsMarkedError(err, pgerror.ContextCanceledMarkerError), + pgerror.IsMarkedError(err, pgerror.ContextDeadlineExceededMarkerError), + pgerror.IsMarkedError(err, errExistingSchemaChangeLease), + pgerror.IsMarkedError(err, errExpiredSchemaChangeLease), + pgerror.IsMarkedError(err, errNotHitGCTTLDeadline), + pgerror.IsMarkedError(err, errSchemaChangeDuringDrain), + pgerror.IsMarkedError(err, errSchemaChangeNotFirstInLine), + pgerror.IsMarkedError(err, exampleTableVersionMismatchError): return false } - switch err := err.(type) { - case errTableVersionMismatch: - return false - case *pgerror.Error: - switch err.Code { - case pgerror.CodeSerializationFailureError, pgerror.CodeConnectionFailureError: + if pgErr, ok := err.(*pgerror.Error); ok { + switch { + case + pgErr.Code == pgerror.CodeSerializationFailureError, + pgErr.Code == pgerror.CodeConnectionFailureError: return false - - case pgerror.CodeInternalError: - if strings.Contains(err.Message, context.DeadlineExceeded.Error()) { - return false - } } } @@ -202,33 +197,36 @@ func isPermanentSchemaChangeError(err error) bool { } var ( - errExistingSchemaChangeLease = pgerror.NewErrorf(pgerror.CodeDataExceptionError, "an outstanding schema change lease exists") - errExpiredSchemaChangeLease = pgerror.NewErrorf(pgerror.CodeDataExceptionError, "the schema change lease has expired") - errSchemaChangeNotFirstInLine = pgerror.NewErrorf(pgerror.CodeDataExceptionError, "schema change not first in line") - errNotHitGCTTLDeadline = pgerror.NewErrorf(pgerror.CodeDataExceptionError, "not hit gc ttl deadline") - errSchemaChangeDuringDrain = pgerror.NewErrorf(pgerror.CodeDataExceptionError, "a schema change ran during the drain phase, re-increment") + errExistingSchemaChangeLease = pgerror.WithMarker( + pgerror.NewErrorf(pgerror.CodeDataExceptionError, "an outstanding schema change lease exists")) + errExpiredSchemaChangeLease = pgerror.WithMarker( + pgerror.NewErrorf(pgerror.CodeDataExceptionError, "the schema change lease has expired")) + errSchemaChangeNotFirstInLine = pgerror.WithMarker( + pgerror.NewErrorf(pgerror.CodeDataExceptionError, "schema change not first in line")) + errNotHitGCTTLDeadline = pgerror.WithMarker( + pgerror.NewErrorf(pgerror.CodeDataExceptionError, "not hit gc ttl deadline")) + errSchemaChangeDuringDrain = pgerror.WithMarker( + pgerror.NewErrorf(pgerror.CodeDataExceptionError, "a schema change ran during the drain phase, re-increment")) ) func shouldLogSchemaChangeError(err error) bool { - return err != errExistingSchemaChangeLease && - err != errSchemaChangeNotFirstInLine && - err != errNotHitGCTTLDeadline + return !pgerror.IsMarkedError(err, errExistingSchemaChangeLease) && + !pgerror.IsMarkedError(err, errSchemaChangeNotFirstInLine) && + !pgerror.IsMarkedError(err, errNotHitGCTTLDeadline) } -type errTableVersionMismatch struct { - version sqlbase.DescriptorVersion - expected sqlbase.DescriptorVersion -} +var exampleTableVersionMismatchError = pgerror.WithMarker(&errTableVersionMismatch{}) -func makeErrTableVersionMismatch(version, expected sqlbase.DescriptorVersion) error { - return errors.WithStack(errTableVersionMismatch{ - version: version, - expected: expected, - }) -} +type errTableVersionMismatch struct{} -func (e errTableVersionMismatch) Error() string { - return fmt.Sprintf("table version mismatch: %d, expected: %d", e.version, e.expected) +func (*errTableVersionMismatch) Error() string { return "" } + +func makeErrTableVersionMismatch(version, expected sqlbase.DescriptorVersion) error { + return pgerror.WithSameMarker( + pgerror.NewErrorf(pgerror.CodeObjectNotInPrerequisiteStateError, + "table version mismatch: %d, expected: %d", + version, expected), + exampleTableVersionMismatchError) } // AcquireLease acquires a schema change lease on the table if @@ -1592,7 +1590,7 @@ func (s *SchemaChangeManager) Start(stopper *stop.Stopper) { if shouldLogSchemaChangeError(err) { log.Warningf(ctx, "Error executing schema change: %s", err) } - if err == sqlbase.ErrDescriptorNotFound { + if pgerror.IsMarkedError(err, sqlbase.ErrDescriptorNotFound) { // Someone deleted this table. Don't try to run the schema // changer again. Note that there's no gossip update for the // deletion which would remove this schemaChanger. diff --git a/pkg/sql/schema_changer_test.go b/pkg/sql/schema_changer_test.go index d66e63c9dac9..2d481f086188 100644 --- a/pkg/sql/schema_changer_test.go +++ b/pkg/sql/schema_changer_test.go @@ -37,6 +37,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/distsqlrun" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/sql/stats" @@ -3133,7 +3134,7 @@ CREATE TABLE t.test (k INT PRIMARY KEY, v INT, pi DECIMAL REFERENCES t.pi (d) DE _, err = sqlbase.GetTableDescFromID(ctx, txn, tableDesc.ID) return err }); err != nil { - if err == sqlbase.ErrDescriptorNotFound { + if pgerror.IsMarkedError(err, sqlbase.ErrDescriptorNotFound) { return nil } return err diff --git a/pkg/sql/set_zone_config.go b/pkg/sql/set_zone_config.go index 5b602b8cf14e..ebf19516aa13 100644 --- a/pkg/sql/set_zone_config.go +++ b/pkg/sql/set_zone_config.go @@ -304,7 +304,7 @@ func (n *setZoneConfigNode) startExec(params runParams) error { _, completeZone, completeSubzone, err := GetZoneConfigInTxn(params.ctx, params.p.txn, uint32(targetID), index, partition, n.setDefault) - if err == errNoZoneConfigApplies { + if pgerror.IsMarkedError(err, errNoZoneConfigApplies) { // No zone config yet. // // GetZoneConfigInTxn will fail with errNoZoneConfigApplies when diff --git a/pkg/sql/show_zone_config.go b/pkg/sql/show_zone_config.go index 1754f5bfcd46..1425f38a5b6f 100644 --- a/pkg/sql/show_zone_config.go +++ b/pkg/sql/show_zone_config.go @@ -22,6 +22,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/config" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/sql/lex" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sem/types" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" @@ -107,7 +108,7 @@ func (n *showZoneConfigNode) startExec(params runParams) error { zoneID, zone, subzone, err := GetZoneConfigInTxn(params.ctx, params.p.txn, uint32(targetID), index, partition, false /* getInheritedDefault */) - if err == errNoZoneConfigApplies { + if pgerror.IsMarkedError(err, errNoZoneConfigApplies) { // TODO(benesch): This shouldn't be the caller's responsibility; // GetZoneConfigInTxn should just return the default zone config if no zone // config applies. diff --git a/pkg/sql/sqlbase/structured.go b/pkg/sql/sqlbase/structured.go index bd8ba01cb457..7f301bd151f8 100644 --- a/pkg/sql/sqlbase/structured.go +++ b/pkg/sql/sqlbase/structured.go @@ -176,12 +176,12 @@ func (dir IndexDescriptor_Direction) ToEncodingDirection() (encoding.Direction, // ErrDescriptorNotFound is returned by GetTableDescFromID to signal that a // descriptor could not be found with the given id. -var ErrDescriptorNotFound = errors.New("descriptor not found") +var ErrDescriptorNotFound = pgerror.WithMarker(errors.New("descriptor not found")) // ErrIndexGCMutationsList is returned by FindIndexByID to signal that the // index with the given ID does not have a descriptor and is in the garbage // collected mutations list. -var ErrIndexGCMutationsList = errors.New("index in GC mutations list") +var ErrIndexGCMutationsList = pgerror.WithMarker(errors.New("index in GC mutations list")) // NewMutableCreatedTableDescriptor returns a MutableTableDescriptor from the // given TableDescriptor with the cluster version being the zero table. This diff --git a/pkg/sql/table.go b/pkg/sql/table.go index 8e7a93b5a64a..1d53488ed85b 100644 --- a/pkg/sql/table.go +++ b/pkg/sql/table.go @@ -108,8 +108,8 @@ func (p *planner) getVirtualTabler() VirtualTabler { return p.extendedEvalCtx.VirtualSchemas } -var errTableDropped = errors.New("table is being dropped") -var errTableAdding = errors.New("table is being added") +var errTableDropped = pgerror.WithMarker(errors.New("table is being dropped")) +var errTableAdding = pgerror.WithMarker(errors.New("table is being added")) func filterTableState(tableDesc *sqlbase.TableDescriptor) error { switch { @@ -336,7 +336,7 @@ func (tc *TableCollection) getTableVersion( // Read the descriptor from the store in the face of some specific errors // because of a known limitation of AcquireByName. See the known // limitations of AcquireByName for details. - if err == errTableDropped || err == sqlbase.ErrDescriptorNotFound { + if pgerror.IsMarkedError(err, errTableDropped) || pgerror.IsMarkedError(err, sqlbase.ErrDescriptorNotFound) { return readTableFromStore() } // Lease acquisition failed with some other error. This we don't @@ -400,7 +400,7 @@ func (tc *TableCollection) getTableVersionByID( origTimestamp := txn.OrigTimestamp() table, expiration, err := tc.leaseMgr.Acquire(ctx, origTimestamp, tableID) if err != nil { - if err == sqlbase.ErrDescriptorNotFound { + if pgerror.IsMarkedError(err, sqlbase.ErrDescriptorNotFound) { // Transform the descriptor error into an error that references the // table's ID. return nil, sqlbase.NewUndefinedRelationError( @@ -607,7 +607,7 @@ func (tc *TableCollection) getUncommittedTable( if mutTbl.Name == string(tn.TableName) && mutTbl.ParentID == dbID { // Right state? - if err = filterTableState(mutTbl.TableDesc()); err != nil && err != errTableAdding { + if err = filterTableState(mutTbl.TableDesc()); err != nil && !pgerror.IsMarkedError(err, errTableAdding) { if !required { // If it's not required here, we simply say we don't have it. err = nil diff --git a/pkg/sql/zone_config.go b/pkg/sql/zone_config.go index 98d80678890e..71f521fd14a5 100644 --- a/pkg/sql/zone_config.go +++ b/pkg/sql/zone_config.go @@ -35,7 +35,7 @@ func init() { config.ZoneConfigHook = ZoneConfigHook } -var errNoZoneConfigApplies = errors.New("no zone config applies") +var errNoZoneConfigApplies = pgerror.WithMarker(errors.New("no zone config applies")) // getZoneConfig recursively looks up entries in system.zones until an // entry that applies to the object with the specified id is @@ -158,7 +158,7 @@ func ZoneConfigHook( } zoneID, zone, _, placeholder, err := getZoneConfig( id, getKey, false /* getInheritedDefault */) - if err == errNoZoneConfigApplies { + if pgerror.IsMarkedError(err, errNoZoneConfigApplies) { return nil, nil, true, nil } else if err != nil { return nil, nil, false, err @@ -272,12 +272,13 @@ func (p *planner) resolveTableForZone( return res, err } +var errMissingKey = pgerror.WithMarker(errors.New("missing key")) + // resolveZone resolves a zone specifier to a zone ID. If the zone // specifier points to a table, index or partition, the table part // must be properly normalized already. It is the caller's // responsibility to do this using e.g .resolveTableForZone(). func resolveZone(ctx context.Context, txn *client.Txn, zs *tree.ZoneSpecifier) (sqlbase.ID, error) { - errMissingKey := errors.New("missing key") id, err := config.ResolveZoneSpecifier(zs, func(parentID uint32, name string) (uint32, error) { kv, err := txn.Get(ctx, sqlbase.MakeNameMetadataKey(sqlbase.ID(parentID), name)) @@ -295,7 +296,7 @@ func resolveZone(ctx context.Context, txn *client.Txn, zs *tree.ZoneSpecifier) ( }, ) if err != nil { - if err == errMissingKey { + if pgerror.IsMarkedError(err, errMissingKey) { return 0, zoneSpecifierNotFoundError(*zs) } return 0, err