Skip to content

Commit

Permalink
sql/types: types.EnumMetadata implements encoding.TextMarshaler inter…
Browse files Browse the repository at this point in the history
…face

Fixes cockroachdb#63379
`types.EnumMetadata` needs to implement `encoding.TextMarshaler`
interface so that goto/protobuf won't panic when text marshaling
protobuf struct has child field of type `types.EnumMetadata`.
See the issue for more details why it would fail without this
fix.

Release note: None
  • Loading branch information
chengxiong-ruan committed Nov 11, 2021
1 parent d7e3ceb commit 309f765
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 0 deletions.
3 changes: 3 additions & 0 deletions pkg/sql/types/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,14 @@ go_test(
srcs = [
"minimum_type_version_test.go",
"types_test.go",
"types_text_marshal_test.go",
],
embed = [":types"],
deps = [
"//pkg/clusterversion",
"//pkg/geo/geopb",
"//pkg/sql/catalog/descpb",
"//pkg/sql/catalog/typedesc",
"//pkg/sql/oidext",
"//pkg/util/leaktest",
"//pkg/util/log",
Expand Down
53 changes: 53 additions & 0 deletions pkg/sql/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ package types

import (
"bytes"
"encoding/hex"
"fmt"
"regexp"
"runtime/debug"
"strconv"
"strings"

"github.com/cockroachdb/cockroach/pkg/geo/geopb"
Expand Down Expand Up @@ -213,6 +215,57 @@ type EnumMetadata struct {
// should occur, if at all.
}

// MarshalText implements the encoding.TextMarshaler interface. This is required
// to make gogo/protobuf able to text marshal any protobuf struct which has
// direct/indirect child field of EnumMetadata type, such as
// descpb.ColumnDescriptor from which you can track down to a EnumMetadata field
// through a field named "type" of types.T.
func (e *EnumMetadata) MarshalText() (text []byte, err error) {
var bf bytes.Buffer

// Convert PhysicalRepresentations to byte slice representation of string like
// "PhysicalRepresentations<[0x12,0x34],[0x56]>".
bf.WriteString("PhysicalRepresentations<")
for i, bs := range e.PhysicalRepresentations {
if i > 0 {
bf.WriteString(",")
}
bf.WriteString("[")
for j := 0; j < len(bs); j++ {
if j > 0 {
bf.WriteString(",")
}
bf.WriteString("0x")
bf.WriteString(hex.EncodeToString(bs[j : j+1]))
}
bf.WriteString("]")
}
bf.WriteString(">,")

// Convert LogicalRepresentations to byte array representation of string like
// "LogicalRepresentations<str1,str2>".
bf.WriteString("LogicalRepresentations<")
for i, str := range e.LogicalRepresentations {
if i > 0 {
bf.WriteString(",")
}
bf.WriteString(str)
}
bf.WriteString(">,")

// Convert IsMemberReadOnly to byte array representation of string like
// "IsMemberReadOnly<true,false>".
bf.WriteString("IsMemberReadOnly<")
for i, bv := range e.IsMemberReadOnly {
if i > 0 {
bf.WriteString(",")
}
bf.WriteString(strconv.FormatBool(bv))
}
bf.WriteString(">")
return bf.Bytes(), nil
}

func (e *EnumMetadata) debugString() string {
return fmt.Sprintf(
"PhysicalReps: %v; LogicalReps: %s",
Expand Down
47 changes: 47 additions & 0 deletions pkg/sql/types/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1092,3 +1092,50 @@ func TestCanonicalType(t *testing.T) {
}
}
}

func TestEnumMetadata_MarshalText(t *testing.T) {
testCases := []struct {
testDesc string
metadata EnumMetadata
expectedStr string
}{
{
"with non-empty slices",
EnumMetadata{
PhysicalRepresentations: [][]byte{
{0x12, 0x34},
{0x56},
},
LogicalRepresentations: []string{"ev1", "ev2"},
IsMemberReadOnly: []bool{true, false},
},
"PhysicalRepresentations<[0x12,0x34],[0x56]>,LogicalRepresentations<ev1,ev2>,IsMemberReadOnly<true,false>",
},
{
"with empty slices",
EnumMetadata{
PhysicalRepresentations: [][]byte{},
LogicalRepresentations: []string{},
IsMemberReadOnly: []bool{},
},
"PhysicalRepresentations<>,LogicalRepresentations<>,IsMemberReadOnly<>",
},
{
"with nil slices",
EnumMetadata{
PhysicalRepresentations: nil,
LogicalRepresentations: nil,
IsMemberReadOnly: nil,
},
"PhysicalRepresentations<>,LogicalRepresentations<>,IsMemberReadOnly<>",
},
}

for _, testCase := range testCases {
t.Run(testCase.testDesc, func(t *testing.T) {
textRpr, err := testCase.metadata.MarshalText()
require.Nil(t, err)
require.Equal(t, testCase.expectedStr, string(textRpr))
})
}
}
47 changes: 47 additions & 0 deletions pkg/sql/types/types_text_marshal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright 2021 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 types_test

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"
)

// TestDescriptorProtoString is to make sure gogo/protobuf is able to text
// marshal a protobuf struct has child field of type EnumMetadata
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() })
}

0 comments on commit 309f765

Please sign in to comment.