Fix problems when altering a column from binary to varbinary

[jorendorff]

Description

Fixes #1627.

The issue is that binary values are incorrectly read from the binlog. This PR fixes it by changing a previously-implemented fix to apply to all binary columns, not only primary keys.

Two new tests are added: a unit test for the function being changed, and a localtest. (Oddly, the existing test in localtests/varbinary doesn't seem to actually involve any varbinary columns.)

• contributed code is using same conventions as original code
• script/cibuild returns with no formatting errors, build errors or unit test errors.

I wasn't able to figure out how to run script/build or script/cibuild. They both try to download and use a vendored Go installation, but this doesn't work for me. Figured it out, it was trivial - my version of Go was too new.

[Copilot]

Pull request overview

This PR addresses incorrect handling of BINARY(N) values read from MySQL binlog events during migrations that alter a column to VARBINARY(M), where trailing 0x00 bytes can be stripped and thus written incorrectly to the ghost table. The fix generalizes the existing padding logic so it applies to all binary columns, not just unique-key columns.

Changes:

• Apply trailing-zero padding for all BinaryColumnType columns during argument conversion (not only unique key columns).
• Add unit tests covering binary padding behavior in convertArg.
• Add a new local integration test (localtests/binary-to-varbinary) to reproduce the binary -> varbinary binlog truncation scenario.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
go/sql/types.go	Expands binary padding logic to apply to all binary columns.
go/sql/types_test.go	Adds unit tests verifying padding behavior for truncated/full binary values.
localtests/binary-to-varbinary/create.sql	Creates a repro scenario with binlog-driven inserts/updates involving trailing-zero binary values.
localtests/binary-to-varbinary/extra_args	Supplies an --alter that changes data from binary(20) to varbinary(32).

Comments suppressed due to low confidence (1)

go/sql/types.go:87

• The padding logic builds a bytes.Buffer directly from arg2Bytes. Since bytes.NewBuffer can reuse the provided slice as its backing store, Write may mutate the input byte slice in-place (depending on capacity). Now that this code runs for all binary columns, it would be safer to avoid mutating the input by copying into a new buffer/slice before padding (and ideally keep the return type as []byte to avoid unnecessary string conversions).

				buf := bytes.NewBuffer(arg2Bytes)
				for i := uint(0); i < (this.BinaryOctetLength - uint(size)); i++ {
					buf.Write([]byte{0})
				}
				arg = buf.String()

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[meiji163]

Thanks for the fix. I do wonder why the author of #915 chose to apply the padding only to the unique key 🤔

[look]

Thanks for the fix. I do wonder why the author of #915 chose to apply the padding only to the unique key 🤔

I think this comment has a clue:

Filling a high number zero bytes may affect processing efficiency. after thinking about it, we can fix it more lighter as below, other scenarios will not cause data consistency problems. 40bc5ae

• only handle the binary column in the unique key
• only handle update/delete dml event

Guess that wasn't quite true...