time "-00:00:00.01" parse value is not right

[rudy-gao]

Hello,

When I use go-mysql version 1.4.0, then I insert "-00:00:00.01" to time(2) type ，but parse binlog value is "00:00:00"

mysql>  select version();
+---------------+
| version()     |
+---------------+
| 5.7.25-28-log |
+---------------+
1 row in set (0.00 sec)

CREATE TABLE `t1` (
  `id` int(11) NOT NULL,
  `a_time` time(2) DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

mysql> insert into t1(id,a_time) values(1,'-00:00:00.01');
Query OK, 1 row affected (0.20 sec)

mysql> select * from t1;
+----+--------------+
| id | a_time       |
+----+--------------+
|  1 | -00:00:00.01 |
+----+--------------+

=== WriteRowsEventV2 ===
Date: 2022-02-11 19:29:08
Log position: 1533
Event size: 44
TableID: 765
Flags: 1
Column count: 2
Values:
0:1
1:"00:00:00"

[lance6716]

thanks for reporting, I can reproduce it.

I'll take a look at my spare time, can you help to check what's wrong with the source code? maybe

go-mysql/replication/row_event.go

Lines 1098 to 1107 in d166653

	case MYSQL_TYPE_TIME:
	n = 3
	i32 := uint32(FixedLengthInt(data[0:3]))
	if i32 == 0 {
	v = "00:00:00"
	} else {
	v = fmt.Sprintf("%02d:%02d:%02d", i32/10000, (i32%10000)/100, i32%100)
	}
	case MYSQL_TYPE_TIME2:
	v, n, err = decodeTime2(data, meta)

is a good entry

[rudy-gao]

I test it will go to code

case MYSQL_TYPE_TIME2:
	v, n, err = decodeTime2(data, meta)

when intPart=-1, then it will intPart++ to intPart=0

func decodeTime2(data []byte, dec uint16) (string, int, error) {
		if intPart < 0 && frac != 0 {
			intPart++     /* Shift to the next integer value */
			frac -= 0x100 /* -(0x100 - frac) */
		}

[lance6716]

@rudy-gao Thanks. I now know we forget to check the frac part in

go-mysql/replication/row_event.go

Lines 1509 to 1511 in d166653

	if intPart == 0 {
	return "00:00:00", n, nil
	}

and

go-mysql/replication/row_event.go

Lines 1408 to 1410 in d166653

	if intPart == 0 {
	return formatZeroTime(int(frac), int(dec)), n, nil
	}

Do you have time to file a PR to fix it? Also some unit tests for decodeTime2 and decodeDatetime2 is needed, you can generate the binlog in upstream, add some fmt.Printf to see the arguments and build test cases.

[rudy-gao]

@lance6716 For bug I modify decodeTime2 and timeFormat func, then add test cases and pass. Sorry, for my personal reason, I can not commit a PR, so modify code as following:

func decodeTime2(data []byte, dec uint16) (string, int, error) {
	//time  binary length
	n := int(3 + (dec+1)/2)

	tmp := int64(0)
	intPart := int64(0)
	frac := int64(0)
	switch dec {
	case 1, 2:
		intPart = int64(BFixedLengthInt(data[0:3])) - TIMEF_INT_OFS
		frac = int64(data[3])
		if intPart < 0 && frac != 0 {
			/*
			   Negative values are stored with reverse fractional part order,
			   for binary sort compatibility.

			     Disk value  intpart frac   Time value   Memory value
			     800000.00    0      0      00:00:00.00  0000000000.000000
			     7FFFFF.FF   -1      255   -00:00:00.01  FFFFFFFFFF.FFD8F0
			     7FFFFF.9D   -1      99    -00:00:00.99  FFFFFFFFFF.F0E4D0
			     7FFFFF.00   -1      0     -00:00:01.00  FFFFFFFFFF.000000
			     7FFFFE.FF   -1      255   -00:00:01.01  FFFFFFFFFE.FFD8F0
			     7FFFFE.F6   -2      246   -00:00:01.10  FFFFFFFFFE.FE7960

			     Formula to convert fractional part from disk format
			     (now stored in "frac" variable) to absolute value: "0x100 - frac".
			     To reconstruct in-memory value, we shift
			     to the next integer value and then substruct fractional part.
			*/
			intPart++     /* Shift to the next integer value */
			frac -= 0x100 /* -(0x100 - frac) */
		}
		tmp = intPart<<24 + frac*10000
	case 3, 4:
		intPart = int64(BFixedLengthInt(data[0:3])) - TIMEF_INT_OFS
		frac = int64(binary.BigEndian.Uint16(data[3:5]))
		if intPart < 0 && frac != 0 {
			/*
			   Fix reverse fractional part order: "0x10000 - frac".
			   See comments for FSP=1 and FSP=2 above.
			*/
			intPart++       /* Shift to the next integer value */
			frac -= 0x10000 /* -(0x10000-frac) */
		}
		tmp = intPart<<24 + frac*100

	case 5, 6:
		tmp = int64(BFixedLengthInt(data[0:6])) - TIMEF_OFS
		return timeFormat(tmp,dec, n)
	default:
		intPart = int64(BFixedLengthInt(data[0:3])) - TIMEF_INT_OFS
		tmp = intPart << 24
	}

	if intPart == 0 && frac == 0 {
		return "00:00:00", n, nil
	}

	return timeFormat(tmp,dec, n)
}

func timeFormat(tmp int64,dec uint16, n int) (string, int, error) {
	hms := int64(0)
	sign := ""
	if tmp < 0 {
		tmp = -tmp
		sign = "-"
	}

	hms = tmp >> 24

	hour := (hms >> 12) % (1 << 10) /* 10 bits starting at 12th */
	minute := (hms >> 6) % (1 << 6) /* 6 bits starting at 6th   */
	second := hms % (1 << 6)        /* 6 bits starting at 0th   */
	secPart := tmp % (1 << 24)

	if secPart != 0 {
		s:=fmt.Sprintf("%s%02d:%02d:%02d.%06d", sign, hour, minute, second, secPart)
		return s[0 : len(s)-(6-int(dec))],n,nil
	}

	return fmt.Sprintf("%s%02d:%02d:%02d", sign, hour, minute, second), n, nil
}

For test case:

func (_ *testDecodeSuite) TestDecodeTime2(c *C) {
	testcases := []struct {
		data        []byte
		dec         uint16
		getFracTime bool
		expected    string
	}{
		{[]byte("\xb4\x6e\xfb"),0, false, "838:59:59"},
		{[]byte("\x80\xf1\x05"),0,false,"15:04:05"},
		{[]byte("\x80\x00\x00"),0,false,"00:00:00"},
		{[]byte("\x7f\xff\xff"),0,false,"-00:00:01"},
		{[]byte("\x7f\x0e\xfb"),0,false,"-15:04:05"},
		{[]byte("\x4b\x91\x05"),0,false,"-838:59:59"},
		{[]byte("\x7f\xff\xff\xff"),2,true,"-00:00:00.01"},
		{[]byte("\x7f\x0e\xfa\xf4"),2,true,"-15:04:05.12"},
		{[]byte("\x4b\x91\x05\xf4"),2,true,"-838:59:58.12"},
		{[]byte("\x7f\xff\xff\xff\xff"),4,true,"-00:00:00.0001"},
		{[]byte("\x7f\x0e\xfa\xfb\x2d"),4,true,"-15:04:05.1235"},
		{[]byte("\x4b\x91\x05\xfb\x2d"),4,true,"-838:59:58.1235"},
		{[]byte("\x7f\xff\xff\xff\xff\xff"),6,true,"-00:00:00.000001"},
		{[]byte("\x7f\x0e\xfa\xfe\x1d\xc0"),6,true,"-15:04:05.123456"},
		{[]byte("\x4b\x91\x05\xfe\x1d\xc0"),6,true,"-838:59:58.123456"},
	}
	for _, tc := range testcases {
		value, _, err := decodeTime2(tc.data, tc.dec)
		c.Assert(err, IsNil)
		c.Assert(value, Equals, tc.expected)
	}
}

[lance6716]

Thanks @rudy-gao ! I think you have finished all the code work. I'll file a PR soon 👍