Skip to content

Commit 596e87d

Browse files
ggilderclaude
andcommitted
Address PR review feedback on varbinary Warning 1300 fix
- Use MySQLType "varbinary(16)" in tests instead of bare "varbinary", matching the real value produced by information_schema COLUMN_TYPE (which includes length). Guards against future refactors that might switch from substring matching to exact matching. - Correct test comment: MySQLType is populated from COLUMN_TYPE, not data_type. - Broaden types.go comment to say "the connection's charset/collation metadata (often utf8mb4)" since gh-ost's connection charset is configurable via --charset. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 31f81d2 commit 596e87d

2 files changed

Lines changed: 7 additions & 6 deletions

File tree

go/sql/types.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,9 @@ func (this *Column) convertArg(arg interface{}) interface{} {
7373
} else if this.Charset == "" && (strings.Contains(this.MySQLType, "binary") || strings.HasSuffix(this.MySQLType, "blob")) {
7474
// varbinary/binary/blob column: no charset means binary storage. Return []byte so
7575
// the MySQL driver sends MYSQL_TYPE_BLOB (binary) rather than MYSQL_TYPE_VAR_STRING
76-
// (text with utf8mb4 metadata), which would cause MySQL to validate the bytes and
77-
// emit Warning 1300 for byte sequences that are invalid utf8mb4.
76+
// (text with the connection's charset/collation metadata, often utf8mb4), which would
77+
// cause MySQL to validate the bytes and emit Warning 1300 for byte sequences that are
78+
// invalid in that charset.
7879
arg = arg2Bytes
7980
} else {
8081
if encoding, ok := charsetEncodingMap[this.Charset]; ok {

go/sql/types_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,15 @@ func TestConvertArgVarbinaryStringWithInvalidUTF8Bytes(t *testing.T) {
107107

108108
col := Column{
109109
Name: "uuid",
110-
Charset: "", // varbinary has no character set
111-
MySQLType: "varbinary", // set by inspect.go from information_schema data_type
110+
Charset: "", // varbinary has no character set
111+
MySQLType: "varbinary(16)", // set by inspect.go from information_schema COLUMN_TYPE
112112
}
113113

114114
result := col.convertArg(string(rawBytes))
115115

116116
require.IsType(t, []byte{}, result,
117117
"varbinary value from binlog (Go string) must be returned as []byte, not string, "+
118-
"to prevent MySQL driver from sending it with utf8mb4 charset metadata")
118+
"to prevent MySQL driver from sending it with the connection's charset metadata")
119119
require.Equal(t, rawBytes, result.([]byte))
120120
}
121121

@@ -127,7 +127,7 @@ func TestConvertArgVarbinaryBytesWithInvalidUTF8Bytes(t *testing.T) {
127127
col := Column{
128128
Name: "uuid",
129129
Charset: "",
130-
MySQLType: "varbinary",
130+
MySQLType: "varbinary(16)", // set by inspect.go from information_schema COLUMN_TYPE
131131
}
132132

133133
result := col.convertArg(rawBytes)

0 commit comments

Comments
 (0)