-
Notifications
You must be signed in to change notification settings - Fork 4.1k
sql: calling String() on some ColumnDescriptors (and possibly other types) can panic #63379
Copy link
Copy link
Closed
Labels
C-bugCode not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.T-sql-foundationsSQL Foundations Team (formerly SQL Schema + SQL Sessions)SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Description
Describe the problem
(This might be a dupe of #63299)
If we run \set auto_trace=on,kv and then ALTER TABLE foo SET LOCALITY REGIONAL BY ROW, you may receive trace data that shows evidence of a panic:
2021-04-08 18:41:50.999899+00:00:00 | 00:00:00.007879 | Put /Table/3/1/55/2/1 -> %!s(PANIC=String method: reflect.Value.Bytes of non-byte slice) | [n1,client=[::1]:53927,hostnossl,user=root] | ©g356o:356 | flow | 36
The following test is capable of reproducing this particular panic:
import (
"testing"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/stretchr/testify/require"
)
func TestDescriptorProtoString(t *testing.T) {
enumMembers := []string{"hi", "hello"}
enumType := types.MakeEnum(typedesc.TypeIDToOID(500), typedesc.TypeIDToOID(100500))
enumType.TypeMeta = types.UserDefinedTypeMetadata{
Name: &types.UserDefinedTypeName{
Schema: "test",
Name: "greeting",
},
EnumData: &types.EnumMetadata{
LogicalRepresentations: enumMembers,
PhysicalRepresentations: [][]byte{
{0x42, 0x1},
{0x42},
},
IsMemberReadOnly: make([]bool, len(enumMembers)),
},
}
desc := &descpb.ColumnDescriptor{
Name: "c",
ID: 1,
Type: enumType,
}
require.NotPanics(t, func() { _ = desc.String() })
}which will fail with:
=== RUN TestDescriptorProtoString
descriptor_test.go:50:
Error Trace: descriptor_test.go:50
Error: func (assert.PanicTestFunc)(0x533dce0) should not panic
Panic value: reflect.Value.Bytes of non-byte slice
Panic stack: goroutine 40 [running]:
runtime/debug.Stack(0xc000af62c0, 0x5500ba0, 0x5eaab00)
/usr/local/Cellar/go/1.16.2/libexec/src/runtime/debug/stack.go:24 +0x9f
github.com/stretchr/testify/assert.didPanic.func1.1(0xc000af7e20, 0xc000af7e0f, 0xc000af7e10)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/stretchr/testify/assert/assertions.go:1013 +0x6b
panic(0x5500ba0, 0x5eaab00)
/usr/local/Cellar/go/1.16.2/libexec/src/runtime/panic.go:965 +0x1b9
reflect.Value.Bytes(0x54daec0, 0xc000a95360, 0x197, 0x6070a0e49ba95b2b, 0xc00034ab60, 0xc000af6750)
/usr/local/Cellar/go/1.16.2/libexec/src/reflect/value.go:291 +0x118
github.com/gogo/protobuf/proto.(*TextMarshaler).writeAny(0x697497a, 0xc000ad2320, 0x54daec0, 0xc000a95360, 0x197, 0xc000aef200, 0x5fbe050, 0x54daec0)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/gogo/protobuf/proto/text.go:557 +0x62e
github.com/gogo/protobuf/proto.(*TextMarshaler).writeStruct(0x697497a, 0xc000ad2320, 0x561d4e0, 0xc000a95360, 0x199, 0x199, 0x0)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/gogo/protobuf/proto/text.go:453 +0x8e7
github.com/gogo/protobuf/proto.(*TextMarshaler).writeAny(0x697497a, 0xc000ad2320, 0x554bfc0, 0xc000a92470, 0x196, 0xc000aeed00, 0x5fbe050, 0x554bfc0)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/gogo/protobuf/proto/text.go:604 +0x287
github.com/gogo/protobuf/proto.(*TextMarshaler).writeStruct(0x697497a, 0xc000ad2320, 0x561d660, 0xc000a92460, 0x199, 0x199, 0x0)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/gogo/protobuf/proto/text.go:453 +0x8e7
github.com/gogo/protobuf/proto.(*TextMarshaler).writeAny(0x697497a, 0xc000ad2320, 0x561d660, 0xc000a92460, 0x199, 0xc000a88c00, 0x5fbe050, 0x561d660)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/gogo/protobuf/proto/text.go:604 +0x287
github.com/gogo/protobuf/proto.(*TextMarshaler).writeStruct(0x697497a, 0xc000ad2320, 0x55e4980, 0xc000a923c0, 0x199, 0x199, 0x0)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/gogo/protobuf/proto/text.go:453 +0x8e7
github.com/gogo/protobuf/proto.(*TextMarshaler).writeAny(0x697497a, 0xc000ad2320, 0x573c9c0, 0xc000a6bf18, 0x196, 0xc000a88a00, 0x5fbe050, 0x573c9c0)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/gogo/protobuf/proto/text.go:604 +0x287
github.com/gogo/protobuf/proto.(*TextMarshaler).writeStruct(0x697497a, 0xc000ad2320, 0x57013e0, 0xc000a6bf00, 0x199, 0x199, 0x7700a68)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/gogo/protobuf/proto/text.go:453 +0x8e7
github.com/gogo/protobuf/proto.(*TextMarshaler).Marshal(0x697497a, 0x5ec4720, 0xc000ad5a10, 0x5ee7ab0, 0xc000a6bf00, 0x9, 0x22)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/gogo/protobuf/proto/text.go:894 +0x22e
github.com/gogo/protobuf/proto.(*TextMarshaler).Text(0x697497a, 0x5ee7ab0, 0xc000a6bf00, 0x40654db, 0xc000411590)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/gogo/protobuf/proto/text.go:906 +0x6e
github.com/gogo/protobuf/proto.CompactTextString(...)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/gogo/protobuf/proto/text.go:928
github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb.(*ColumnDescriptor).String(...)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb/structured.pb.go:877
github.com/cockroachdb/cockroach/pkg/sql/catalog_test.TestDescriptorProtoString.func1()
/Users/ssd/go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descriptor_test.go:50 +0x45
github.com/stretchr/testify/assert.didPanic.func1(0xc000411620, 0xc00041160f, 0xc000411610, 0xc000ade540)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/stretchr/testify/assert/assertions.go:1018 +0x6c
github.com/stretchr/testify/assert.didPanic(0xc000ade540, 0x2f6d7a38, 0xc0003b3c80, 0x2f6d7a58, 0xc0003b3c80, 0xc0003b3c01)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/stretchr/testify/assert/assertions.go:1020 +0x5f
github.com/stretchr/testify/assert.NotPanics(0x2f6d7a38, 0xc0003b3c80, 0xc000ade540, 0x0, 0x0, 0x0, 0xc000ade540)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/stretchr/testify/assert/assertions.go:1091 +0x77
github.com/stretchr/testify/require.NotPanics(0x5ede130, 0xc0003b3c80, 0xc000ade540, 0x0, 0x0, 0x0)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/vendor/github.com/stretchr/testify/require/require.go:1236 +0xbe
github.com/cockroachdb/cockroach/pkg/sql/catalog_test.TestDescriptorProtoString(0xc0003b3c80)
/Users/ssd/go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descriptor_test.go:50 +0x330
testing.tRunner(0xc0003b3c80, 0x5c87610)
/usr/local/Cellar/go/1.16.2/libexec/src/testing/testing.go:1194 +0xef
created by testing.(*T).Run
/usr/local/Cellar/go/1.16.2/libexec/src/testing/testing.go:1239 +0x2b3
Test: TestDescriptorProtoString
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
C-bugCode not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.T-sql-foundationsSQL Foundations Team (formerly SQL Schema + SQL Sessions)SQL Foundations Team (formerly SQL Schema + SQL Sessions)