Fix binary column trailing zero stripping for non-key columns

MySQL's binlog strips trailing 0x00 bytes from binary(N) columns.
PR #915 fixed this for unique key columns only, but the same issue
affects all binary columns in INSERT/UPDATE operations.

Remove the isUniqueKeyColumn condition so all binary(N) columns
are padded to their declared length.

Fixes a variation of #909 where the affected column is not a primary key.

Author: jorendorff

go/sql/types.go

@@ -77,7 +77,7 @@ func (this *Column) convertArg(arg interface{}, isUniqueKeyColumn bool) interfac
 			}
 		}
 
-		if this.Type == BinaryColumnType && isUniqueKeyColumn {
+		if this.Type == BinaryColumnType {
 			size := len(arg2Bytes)
 			if uint(size) < this.BinaryOctetLength {
 				buf := bytes.NewBuffer(arg2Bytes)

go/sql/types_test.go

@@ -65,3 +65,68 @@ func TestConvertArgCharsetDecoding(t *testing.T) {
 	str := col.convertArg(latin1Bytes, false)
 	require.Equal(t, "Garçon !", str)
 }
+
+func TestConvertArgBinaryColumnPadding(t *testing.T) {
+	// Test that binary columns are padded with trailing zeros to their declared length.
+	// This is needed because MySQL's binlog strips trailing 0x00 bytes from binary values.
+	// See https://github.com/github/gh-ost/issues/909
+
+	// Simulates a binary(20) column where binlog delivered only 18 bytes
+	// (trailing zeros were stripped)
+	truncatedValue := []uint8{
+		0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
+		0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10,
+		0x11, 0x12, // 18 bytes, missing 2 trailing zeros
+	}
+
+	col := Column{
+		Name:              "bin_col",
+		Type:              BinaryColumnType,
+		BinaryOctetLength: 20,
+	}
+
+	// Test with isUniqueKeyColumn=false (the bug was that non-key columns weren't padded)
+	result := col.convertArg(truncatedValue, false)
+	resultBytes := []byte(result.(string))
+
+	require.Equal(t, 20, len(resultBytes), "binary column should be padded to declared length")
+	// First 18 bytes should be unchanged
+	require.Equal(t, truncatedValue, resultBytes[:18])
+	// Last 2 bytes should be zeros
+	require.Equal(t, []byte{0x00, 0x00}, resultBytes[18:])
+
+	// Test with isUniqueKeyColumn=true (this already worked before the fix)
+	resultKey := col.convertArg(truncatedValue, true)
+	resultKeyBytes := []byte(resultKey.(string))
+	require.Equal(t, 20, len(resultKeyBytes), "binary key column should be padded to declared length")
+}
+
+func TestConvertArgBinaryColumnNoPaddingWhenFull(t *testing.T) {
+	// When binary value is already at full length, no padding should occur
+	fullValue := []uint8{
+		0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
+		0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10,
+		0x11, 0x12, 0x13, 0x14, // 20 bytes
+	}
+
+	col := Column{
+		Name:              "bin_col",
+		Type:              BinaryColumnType,
+		BinaryOctetLength: 20,
+	}
+
+	result := col.convertArg(fullValue, false)
+	// When no padding needed, result may be []uint8 or string depending on code path
+	var resultBytes []byte
+	switch v := result.(type) {
+	case string:
+		resultBytes = []byte(v)
+	case []uint8:
+		resultBytes = v
+	default:
+		t.Fatalf("unexpected type %T", result)
+	}
+
+	require.Equal(t, 20, len(resultBytes))
+	require.Equal(t, []byte(fullValue), resultBytes)
+}

localtests/binary-to-varbinary/create.sql

@@ -0,0 +1,38 @@
+-- Test for https://github.com/github/gh-ost/issues/909 variant:
+-- Binary columns with trailing zeros should preserve their values
+-- when migrating from binary(N) to varbinary(M), even for rows
+-- modified during migration via binlog events.
+
+drop table if exists gh_ost_test;
+create table gh_ost_test (
+  id int NOT NULL AUTO_INCREMENT,
+  info varchar(255) NOT NULL,
+  data binary(20) NOT NULL,
+  PRIMARY KEY (id)
+) auto_increment=1;
+
+drop event if exists gh_ost_test;
+delimiter ;;
+create event gh_ost_test
+  on schedule every 1 second
+  starts current_timestamp
+  ends current_timestamp + interval 60 second
+  on completion not preserve
+  enable
+  do
+begin
+  -- Insert rows where data has trailing zeros (will be stripped by binlog)
+  INSERT INTO gh_ost_test (info, data) VALUES ('insert-during-1', X'aabbccdd00000000000000000000000000000000');
+  INSERT INTO gh_ost_test (info, data) VALUES ('insert-during-2', X'11223344556677889900000000000000000000ee');
+
+  -- Update existing rows to values with trailing zeros
+  UPDATE gh_ost_test SET data = X'ffeeddcc00000000000000000000000000000000' WHERE info = 'update-target-1';
+  UPDATE gh_ost_test SET data = X'aabbccdd11111111111111111100000000000000' WHERE info = 'update-target-2';
+end ;;
+
+-- Pre-existing rows (copied via rowcopy, not binlog - these should work fine)
+INSERT INTO gh_ost_test (info, data) VALUES
+  ('pre-existing-1', X'01020304050607080910111213141516171819ff'),
+  ('pre-existing-2', X'0102030405060708091011121314151617181900'),
+  ('update-target-1', X'ffffffffffffffffffffffffffffffffffffffff'),
+  ('update-target-2', X'eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee');

localtests/binary-to-varbinary/extra_args

@@ -0,0 +1 @@
+--alter="MODIFY data varbinary(32)"