Fix zero timestamp

[agonzale34]

Description

This PR contains bugfix for timestamp zero issue we are facing when migrating tables that get new inserts, with 0 timestamp value, which get a wrong format.

In case this PR introduced Go code changes:

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

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[pbitty]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

Where does this come from?

[agonzale34]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

Where does this come from?

no idea, maybe some of the files in .github folder?

[pbitty]

LGTM

Noting here that we went over this in a video call and confirmed that timestamp 1970-01-01 00:00:00 isn't valid for MySQL. The lowest value you can put seems to be 1970-01-01 00:00:01.

[pbitty]

@agonzale34 , good catch.

Regarding contributing this upstream, it appears this problem has been fixed because they updated the version of the go-mysql/replication library they are using in v1.1.6.

It used to be github.com/siddontang/go-mysql/replication and at some point it became github.com/go-mysql-org/go-mysql/replication. In that version they do have a special case for the zero value: https://github.com/github/gh-ost/blob/732a0642317a544aee533f6b2e0d04de08a1d490/vendor/github.com/go-mysql-org/go-mysql/replication/row_event.go#L1048-L1053.

	case MYSQL_TYPE_TIMESTAMP:
		n = 4
		t := binary.LittleEndian.Uint32(data)
		if t == 0 {
			v = formatZeroTime(0, 0)
		} else {
			// ...

[agonzale34]

@agonzale34 , good catch.

Regarding contributing this upstream, it appears this problem has been fixed because they updated the version of the go-mysql/replication library they are using in v1.1.6.

It used to be github.com/siddontang/go-mysql/replication and at some point it became github.com/go-mysql-org/go-mysql/replication. In that version they do have a special case for the zero value: https://github.com/github/gh-ost/blob/732a0642317a544aee533f6b2e0d04de08a1d490/vendor/github.com/go-mysql-org/go-mysql/replication/row_event.go#L1048-L1053.

	case MYSQL_TYPE_TIMESTAMP:
		n = 4
		t := binary.LittleEndian.Uint32(data)
		if t == 0 {
			v = formatZeroTime(0, 0)
		} else {
			// ...

@pbitty Thanks for going above a beyond with that investigation. I really wish we could use this version with our AWS RDS cluster, sadly the tx_isolation error won't let us use the version:
github#1262 (comment)

[pbitty]

I realize that. I just mean that I don't think we need to contribute this fix upstream - it looks like it's been fixed.

It was easy for me to find this because I've worked a lot on our outbox-relay tool, which uses the same library and I could see that gh-ost was using an old version. But it turns out they upgraded it on the latest release.

[agonzale34]

I realize that. I just mean that I don't think we need to contribute this fix upstream - it looks like it's been fixed.

It was easy for me to find this because I've worked a lot on our outbox-relay tool, which uses the same library and I could see that gh-ost was using an old version. But it turns out they upgraded it on the latest release.

Perfect, this PR is just internally for us, while they main project update the issues with the rds.