Skip to content

Commit

Permalink
colexec: use redact.RedactableString for types in cast error message
Browse files Browse the repository at this point in the history
This patch adds a `SQLStringForError` method to `types.T` that returns
a `redact.RedactableString`. It is used by the "unhandled cast" error
message to prevent safe type information from being removed from the
error message. This will make debugging sentry issues easier.

Fixes #90760

Release note: None
  • Loading branch information
DrewKimball committed Mar 16, 2023
1 parent 1d2a647 commit 1a4b094
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 7 deletions.
1 change: 1 addition & 0 deletions pkg/sql/colexec/colexecbase/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ go_library(
"//pkg/util/uuid", # keep
"@com_github_cockroachdb_apd_v3//:apd", # keep
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_redact//:redact", # keep
"@com_github_lib_pq//oid", # keep
],
)
Expand Down
6 changes: 5 additions & 1 deletion pkg/sql/colexec/colexecbase/cast.eg.go

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

6 changes: 5 additions & 1 deletion pkg/sql/colexec/colexecbase/cast_tmpl.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,11 @@ func GetCastOperator(
// {{end}}
}
}
return nil, errors.Errorf("unhandled cast %s -> %s", fromType.SQLString(), toType.SQLString())
return nil, errors.Errorf(
"unhandled cast %s -> %s",
fromType.SQLStringForError(),
toType.SQLStringForError(),
)
}

func IsCastSupported(fromType, toType *types.T) bool {
Expand Down
14 changes: 9 additions & 5 deletions pkg/sql/colmem/allocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,14 +440,14 @@ func (a *Allocator) MaybeAppendColumn(b coldata.Batch, t *types.T, colIdx int) {
// We have a vector with an unexpected type, so we panic.
colexecerror.InternalError(errors.AssertionFailedf(
"trying to add a column of %s type at index %d but %s vector already present",
t.SQLString(), colIdx, presentType.SQLString(),
t.SQLStringForError(), colIdx, presentType.SQLStringForError(),
))
} else if colIdx > width {
// We have a batch of unexpected width which indicates an error in the
// planning stage.
colexecerror.InternalError(errors.AssertionFailedf(
"trying to add a column of %s type at index %d but batch has width %d",
t.SQLString(), colIdx, width,
t.SQLStringForError(), colIdx, width,
))
}
estimatedMemoryUsage := EstimateBatchSizeBytes([]*types.T{t}, desiredCapacity)
Expand Down Expand Up @@ -648,7 +648,7 @@ func EstimateBatchSizeBytes(vecTypes []*types.T, batchLength int) int64 {
// Types that have a statically known size.
acc += GetFixedSizeTypeSize(t)
default:
colexecerror.InternalError(errors.AssertionFailedf("unhandled type %s", t.SQLString()))
colexecerror.InternalError(errors.AssertionFailedf("unhandled type %s", t.SQLStringForError()))
}
}
// For byte arrays, we initially allocate a constant number of bytes for
Expand Down Expand Up @@ -689,7 +689,7 @@ func GetFixedSizeTypeSize(t *types.T) (size int64) {
case types.IntervalFamily:
size = memsize.Duration
default:
colexecerror.InternalError(errors.AssertionFailedf("unhandled type %s", t.SQLString()))
colexecerror.InternalError(errors.AssertionFailedf("unhandled type %s", t.SQLStringForError()))
}
return size
}
Expand Down Expand Up @@ -967,7 +967,11 @@ func (h *SetAccountingHelper) ResetMaybeReallocate(
case types.JsonFamily:
h.bytesLikeVectors = append(h.bytesLikeVectors, &vecs[vecIdx].JSON().Bytes)
default:
colexecerror.InternalError(errors.AssertionFailedf("unexpected bytes-like type: %s", typs[vecIdx].SQLString()))
colexecerror.InternalError(
errors.AssertionFailedf(
"unexpected bytes-like type: %s", typs[vecIdx].SQLStringForError(),
),
)
}
}
h.prevBytesLikeTotalSize = h.getBytesLikeTotalSize()
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/types/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ go_library(
"//pkg/util/errorutil/unimplemented",
"//pkg/util/protoutil",
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_redact//:redact",
"@com_github_gogo_protobuf//jsonpb",
"@com_github_gogo_protobuf//proto",
"@com_github_lib_pq//oid",
Expand Down
36 changes: 36 additions & 0 deletions pkg/sql/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/redact"
"github.com/gogo/protobuf/proto"
"github.com/lib/pq/oid"
)
Expand Down Expand Up @@ -1923,6 +1924,41 @@ func (t *T) SQLString() string {
return strings.ToUpper(t.Name())
}

// SQLStringForError returns a version of SQLString that will preserve safe
// information during redaction. It is suitable for usage in error messages.
func (t *T) SQLStringForError() redact.RedactableString {
if t.UserDefined() {
// Show the redacted SQLString output with an un-redacted prefix to indicate
// that the type is user defined (and possibly enum or record).
prefix := "TYPE"
switch t.Family() {
case EnumFamily:
prefix = "ENUM"
case TupleFamily:
prefix = "RECORD"
case ArrayFamily:
prefix = "ARRAY"
}
return redact.Sprintf("USER DEFINED %s: %s", redact.Safe(prefix), t.SQLString())
}
switch t.Family() {
case EnumFamily, TupleFamily, ArrayFamily:
// These types can be or can contain user-defined types, but the SQLString
// is safe when they are not user-defined. We filtered out the user-defined
// case above.
return redact.Sprint(redact.Safe(t.SQLString()))
case BoolFamily, IntFamily, FloatFamily, DecimalFamily, DateFamily, TimestampFamily,
IntervalFamily, StringFamily, BytesFamily, TimestampTZFamily, CollatedStringFamily, OidFamily,
UnknownFamily, UuidFamily, INetFamily, TimeFamily, JsonFamily, TimeTZFamily, BitFamily,
GeometryFamily, GeographyFamily, Box2DFamily, VoidFamily, EncodedKeyFamily, TSQueryFamily,
TSVectorFamily, AnyFamily:
// These types do not contain other types, and do not require redaction.
return redact.Sprint(redact.SafeString(t.SQLString()))
}
// Default to redaction for unhandled types.
return redact.Sprint(t.SQLString())
}

// FormatTypeName is an injected dependency from tree to properly format a
// type name. The logic for proper formatting lives in the tree package.
var FormatTypeName = fallbackFormatTypeName
Expand Down
71 changes: 71 additions & 0 deletions pkg/sql/types/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1086,3 +1086,74 @@ func TestEnumWithoutTypeMetaNameDoesNotPanicInSQLString(t *testing.T) {
arrayType := MakeArray(typ)
require.Equal(t, "@100100[]", arrayType.SQLString())
}

func TestSQLStringForError(t *testing.T) {
const userDefinedOID = oidext.CockroachPredefinedOIDMax + 500
userDefinedEnum := MakeEnum(userDefinedOID, userDefinedOID+3)
nonUserDefinedEnum := MakeEnum(500, 503)
userDefinedTuple := &T{
InternalType: InternalType{
Family: TupleFamily, Oid: userDefinedOID, TupleContents: []*T{Int}, Locale: &emptyLocale,
},
TypeMeta: UserDefinedTypeMetadata{Name: &UserDefinedTypeName{Name: "foo"}},
}
arrayWithUserDefinedContent := MakeArray(userDefinedEnum)

testCases := []struct {
typ *T
expected string
}{
{ // Case 1: un-redacted
typ: Int,
expected: "INT8",
},
{ // Case 2: un-redacted
typ: Float,
expected: "FLOAT8",
},
{ // Case 3: un-redacted
typ: Decimal,
expected: "DECIMAL",
},
{ // Case 4: un-redacted
typ: String,
expected: "STRING",
},
{ // Case 5: un-redacted
typ: TimestampTZ,
expected: "TIMESTAMPTZ",
},
{ // Case 6: un-redacted
typ: nonUserDefinedEnum,
expected: "@500",
},
{ // Case 7: redacted because user-defined
typ: userDefinedEnum,
expected: "USER DEFINED ENUM: ‹@100500›",
},
{ // Case 8: un-redacted
typ: MakeTuple([]*T{Int, Float}),
expected: "RECORD",
},
{ // Case 9: un-redacted because contents are not visible
typ: MakeTuple([]*T{Int, userDefinedEnum}),
expected: "RECORD",
},
{ // Case 10: redacted because user-defined
typ: userDefinedTuple,
expected: "USER DEFINED RECORD: ‹FOO›",
},
{ // Case 11: un-redacted
typ: MakeArray(Int),
expected: "INT8[]",
},
{ // Case 12: redacted element type
typ: arrayWithUserDefinedContent,
expected: "USER DEFINED ARRAY: ‹@100500[]›",
},
}

for i, tc := range testCases {
require.Equalf(t, tc.expected, string(tc.typ.SQLStringForError()), "test case %d", i+1)
}
}

0 comments on commit 1a4b094

Please sign in to comment.