From bae525808249672288854c884f36a6f77e33b8cb Mon Sep 17 00:00:00 2001 From: lzmhhh123 Date: Tue, 11 Aug 2020 16:07:17 +0800 Subject: [PATCH 1/5] types: fix wrong hash key for decimal --- executor/join_test.go | 12 ++++++++++++ types/mydecimal.go | 7 +++++++ types/mydecimal_test.go | 2 ++ 3 files changed, 21 insertions(+) diff --git a/executor/join_test.go b/executor/join_test.go index 7c6831b7b366c..cf51128b761f3 100644 --- a/executor/join_test.go +++ b/executor/join_test.go @@ -2166,3 +2166,15 @@ func (s *testSuite9) TestIssue18572_3(c *C) { _, err = session.GetRows4Test(context.Background(), nil, rs) c.Assert(strings.Contains(err.Error(), "mockIndexHashJoinBuildErr"), IsTrue) } + +func (s *testSuite9) TestIssue19112(c *C) { + tk := testkit.NewTestKitWithInit(c, s.store) + tk.MustExec("drop table if exists t1, t2") + tk.MustExec("create table t1 ( c_int int, c_decimal decimal(12, 6), key(c_int), unique key(c_decimal) )") + tk.MustExec("create table t2 like t1") + tk.MustExec("insert into t1 (c_int, c_decimal) values (1, 4.064000), (2, 0.257000), (3, 1.010000)") + tk.MustExec("insert into t2 (c_int, c_decimal) values (1, 4.064000), (3, 1.010000)") + tk.MustQuery("select /*+ HASH_JOIN(t1,t2) */ * from t1 join t2 on t1.c_decimal = t2.c_decimal order by t1.c_int").Check(testkit.Rows( + "1 4.064000 1 4.064000", + "3 1.010000 3 1.010000")) +} diff --git a/types/mydecimal.go b/types/mydecimal.go index c8999a567c45d..d66929e9dcd1f 100644 --- a/types/mydecimal.go +++ b/types/mydecimal.go @@ -14,6 +14,7 @@ package types import ( + "encoding/binary" "math" "strconv" @@ -1301,6 +1302,12 @@ func (d *MyDecimal) ToHashKey() ([]byte, error) { // thus ErrTruncated may be raised, we can ignore it here. err = nil } + precBuff := make([]byte, 4) + digitBuff := make([]byte, 4) + binary.LittleEndian.PutUint32(precBuff, uint32(prec)) + binary.LittleEndian.PutUint32(digitBuff, uint32(digitsFrac)) + buf = append(buf, precBuff...) + buf = append(buf, digitBuff...) return buf, err } diff --git a/types/mydecimal_test.go b/types/mydecimal_test.go index 576867f7de963..310e40c0615a2 100644 --- a/types/mydecimal_test.go +++ b/types/mydecimal_test.go @@ -208,6 +208,8 @@ func (s *testMyDecimalSuite) TestToHashKey(c *C) { var dec MyDecimal c.Check(dec.FromString([]byte(num)), IsNil) key, err := dec.ToHashKey() + // remove prec and digit len + key = key[:len(key)-8] c.Check(err, IsNil) keys = append(keys, string(key)) } From a32486727a41b25317a9f90db039774a34c3fea9 Mon Sep 17 00:00:00 2001 From: lzmhhh123 Date: Wed, 12 Aug 2020 11:34:00 +0800 Subject: [PATCH 2/5] fix bug --- types/mydecimal.go | 2 +- types/mydecimal_test.go | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/types/mydecimal.go b/types/mydecimal.go index d66929e9dcd1f..b48536bf50cd9 100644 --- a/types/mydecimal.go +++ b/types/mydecimal.go @@ -1228,7 +1228,7 @@ func (d *MyDecimal) WriteBin(precision, frac int, buf []byte) ([]byte, error) { } } - if fracSize < fracSizeFrom { + if fracSize < fracSizeFrom || (fracSize == fracSizeFrom && (trailingDigits < trailingDigitsFrom || wordsFrac < wordsFrac)) { wordsFracFrom = wordsFrac trailingDigitsFrom = trailingDigits err = ErrTruncated diff --git a/types/mydecimal_test.go b/types/mydecimal_test.go index 310e40c0615a2..5fcefc1cf5697 100644 --- a/types/mydecimal_test.go +++ b/types/mydecimal_test.go @@ -573,6 +573,13 @@ func (s *testMyDecimalSuite) TestToBinFromBin(c *C) { {"000000000.01", 7, 3, "0.010", nil}, {"123.4", 10, 2, "123.40", nil}, {"1000", 3, 0, "0", ErrOverflow}, + {"0.1", 1, 1, "0.1", nil}, + {"0.100", 1, 1, "0.1", ErrTruncated}, + {"0.1000", 1, 1, "0.1", ErrTruncated}, + {"0.10000", 1, 1, "0.1", ErrTruncated}, + {"0.100000", 1, 1, "0.1", ErrTruncated}, + {"0.1000000", 1, 1, "0.1", ErrTruncated}, + {"0.10", 1, 1, "0.1", ErrTruncated}, } for _, ca := range tests { var dec MyDecimal From d6531041a710f0d1914d99de18c914caf5a79aa3 Mon Sep 17 00:00:00 2001 From: lzmhhh123 Date: Wed, 12 Aug 2020 14:12:03 +0800 Subject: [PATCH 3/5] fix & add test --- types/mydecimal.go | 7 +++++-- types/mydecimal_test.go | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/types/mydecimal.go b/types/mydecimal.go index b48536bf50cd9..8cfcdd20ff6ca 100644 --- a/types/mydecimal.go +++ b/types/mydecimal.go @@ -1228,10 +1228,13 @@ func (d *MyDecimal) WriteBin(precision, frac int, buf []byte) ([]byte, error) { } } - if fracSize < fracSizeFrom || (fracSize == fracSizeFrom && (trailingDigits < trailingDigitsFrom || wordsFrac < wordsFrac)) { + if fracSize < fracSizeFrom || + (fracSize == fracSizeFrom && (trailingDigits <= trailingDigitsFrom || wordsFrac <= wordsFracFrom)) { + if fracSize < fracSizeFrom || (fracSize == fracSizeFrom && trailingDigits < trailingDigitsFrom) || (fracSize == fracSizeFrom && wordsFrac < wordsFracFrom) { + err = ErrTruncated + } wordsFracFrom = wordsFrac trailingDigitsFrom = trailingDigits - err = ErrTruncated } else if fracSize > fracSizeFrom && trailingDigitsFrom > 0 { if wordsFrac == wordsFracFrom { trailingDigitsFrom = trailingDigits diff --git a/types/mydecimal_test.go b/types/mydecimal_test.go index 5fcefc1cf5697..61e83bbb46634 100644 --- a/types/mydecimal_test.go +++ b/types/mydecimal_test.go @@ -580,6 +580,10 @@ func (s *testMyDecimalSuite) TestToBinFromBin(c *C) { {"0.100000", 1, 1, "0.1", ErrTruncated}, {"0.1000000", 1, 1, "0.1", ErrTruncated}, {"0.10", 1, 1, "0.1", ErrTruncated}, + {"0000000000000000000000000000000000000000000.000000000000123000000000000000", 15, 15, "0.000000000000123", ErrTruncated}, + {"00000000000000000000000000000.00000000000012300", 15, 15, "0.000000000000123", ErrTruncated}, + {"0000000000000000000000000000000000000000000.0000000000001234000000000000000", 16, 16, "0.0000000000001234", ErrTruncated}, + {"00000000000000000000000000000.000000000000123400", 16, 16, "0.0000000000001234", ErrTruncated}, } for _, ca := range tests { var dec MyDecimal From ea4db38a54aa2d8f79f056a254782a71351e5ea4 Mon Sep 17 00:00:00 2001 From: lzmhhh123 Date: Thu, 13 Aug 2020 11:39:17 +0800 Subject: [PATCH 4/5] address comments and add test --- executor/aggregate_test.go | 12 ++++++++++++ types/mydecimal.go | 8 +------- types/mydecimal_test.go | 9 +++++++-- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/executor/aggregate_test.go b/executor/aggregate_test.go index 24621071297fc..00f687ceab4a3 100644 --- a/executor/aggregate_test.go +++ b/executor/aggregate_test.go @@ -1081,3 +1081,15 @@ func (s *testSuiteAgg) TestIssue15958(c *C) { tk.MustQuery(`select sum(y) from t`).Check(testkit.Rows("6070")) tk.MustQuery(`select avg(y) from t`).Check(testkit.Rows("2023.3333")) } + +func (s *testSuiteAgg) TestIssue17216(c *C) { + tk := testkit.NewTestKitWithInit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t1") + tk.MustExec(`CREATE TABLE t1 ( + pk int(11) NOT NULL, + col1 decimal(40,20) DEFAULT NULL + )`) + tk.MustExec(`INSERT INTO t1 VALUES (2084,0.02040000000000000000),(35324,0.02190000000000000000),(43760,0.00510000000000000000),(46084,0.01400000000000000000),(46312,0.00560000000000000000),(61632,0.02730000000000000000),(94676,0.00660000000000000000),(102244,0.01810000000000000000),(113144,0.02140000000000000000),(157024,0.02750000000000000000),(157144,0.01750000000000000000),(182076,0.02370000000000000000),(188696,0.02330000000000000000),(833,0.00390000000000000000),(6701,0.00230000000000000000),(8533,0.01690000000000000000),(13801,0.01360000000000000000),(20797,0.00680000000000000000),(36677,0.00550000000000000000),(46305,0.01290000000000000000),(76113,0.00430000000000000000),(76753,0.02400000000000000000),(92393,0.01720000000000000000),(111733,0.02690000000000000000),(152757,0.00250000000000000000),(162393,0.02760000000000000000),(167169,0.00440000000000000000),(168097,0.01360000000000000000),(180309,0.01720000000000000000),(19918,0.02620000000000000000),(58674,0.01820000000000000000),(67454,0.01510000000000000000),(70870,0.02880000000000000000),(89614,0.02530000000000000000),(106742,0.00180000000000000000),(107886,0.01580000000000000000),(147506,0.02230000000000000000),(148366,0.01340000000000000000),(167258,0.01860000000000000000),(194438,0.00500000000000000000),(10307,0.02850000000000000000),(14539,0.02210000000000000000),(27703,0.00050000000000000000),(32495,0.00680000000000000000),(39235,0.01450000000000000000),(52379,0.01640000000000000000),(54551,0.01910000000000000000),(85659,0.02330000000000000000),(104483,0.02670000000000000000),(109911,0.02040000000000000000),(114523,0.02110000000000000000),(119495,0.02120000000000000000),(137603,0.01910000000000000000),(154031,0.02580000000000000000);`) + tk.MustQuery("SELECT count(distinct col1) FROM t1").Check(testkit.Rows("48")) +} diff --git a/types/mydecimal.go b/types/mydecimal.go index 8cfcdd20ff6ca..920da505d4960 100644 --- a/types/mydecimal.go +++ b/types/mydecimal.go @@ -14,7 +14,6 @@ package types import ( - "encoding/binary" "math" "strconv" @@ -1305,12 +1304,7 @@ func (d *MyDecimal) ToHashKey() ([]byte, error) { // thus ErrTruncated may be raised, we can ignore it here. err = nil } - precBuff := make([]byte, 4) - digitBuff := make([]byte, 4) - binary.LittleEndian.PutUint32(precBuff, uint32(prec)) - binary.LittleEndian.PutUint32(digitBuff, uint32(digitsFrac)) - buf = append(buf, precBuff...) - buf = append(buf, digitBuff...) + buf = append(buf, byte(digitsFrac)) return buf, err } diff --git a/types/mydecimal_test.go b/types/mydecimal_test.go index 61e83bbb46634..8799cedf3061b 100644 --- a/types/mydecimal_test.go +++ b/types/mydecimal_test.go @@ -208,8 +208,8 @@ func (s *testMyDecimalSuite) TestToHashKey(c *C) { var dec MyDecimal c.Check(dec.FromString([]byte(num)), IsNil) key, err := dec.ToHashKey() - // remove prec and digit len - key = key[:len(key)-8] + // remove digit len + key = key[:len(key)-1] c.Check(err, IsNil) keys = append(keys, string(key)) } @@ -584,6 +584,11 @@ func (s *testMyDecimalSuite) TestToBinFromBin(c *C) { {"00000000000000000000000000000.00000000000012300", 15, 15, "0.000000000000123", ErrTruncated}, {"0000000000000000000000000000000000000000000.0000000000001234000000000000000", 16, 16, "0.0000000000001234", ErrTruncated}, {"00000000000000000000000000000.000000000000123400", 16, 16, "0.0000000000001234", ErrTruncated}, + {"0.1", 2, 2, "0.10", nil}, + {"0.10", 3, 3, "0.100", nil}, + {"0.1", 3, 1, "0.1", nil}, + {"0.0000000000001234", 32, 17, "0.00000000000012340", nil}, + {"0.0000000000001234", 20, 20, "0.00000000000012340000", nil}, } for _, ca := range tests { var dec MyDecimal From fd12e6b6b8d5ee51279b1d8a1e3c9b8e432e82f2 Mon Sep 17 00:00:00 2001 From: lzmhhh123 Date: Thu, 13 Aug 2020 19:18:27 +0800 Subject: [PATCH 5/5] add test --- types/mydecimal_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/mydecimal_test.go b/types/mydecimal_test.go index 8799cedf3061b..4b4a31f4f6411 100644 --- a/types/mydecimal_test.go +++ b/types/mydecimal_test.go @@ -157,7 +157,7 @@ func (s *testMyDecimalSuite) TestToHashKey(c *C) { }{ {[]string{"1.1", "1.1000", "1.1000000", "1.10000000000", "01.1", "0001.1", "001.1000000"}}, {[]string{"-1.1", "-1.1000", "-1.1000000", "-1.10000000000", "-01.1", "-0001.1", "-001.1000000"}}, - {[]string{".1", "0.1", "000000.1", ".10000", "0000.10000", "000000000000000000.1"}}, + {[]string{".1", "0.1", "0.10", "000000.1", ".10000", "0000.10000", "000000000000000000.1"}}, {[]string{"0", "0000", ".0", ".00000", "00000.00000", "-0", "-0000", "-.0", "-.00000", "-00000.00000"}}, {[]string{".123456789123456789", ".1234567891234567890", ".12345678912345678900", ".123456789123456789000", ".1234567891234567890000", "0.123456789123456789", ".1234567891234567890000000000", "0000000.123456789123456789000"}},