types: fix wrong hash key for decimal

[lzmhhh123]

What problem does this PR solve?

Issue Number: close #19112

What is changed and how it works?

What's Changed: MyDecimal.ToHashKey should consider the length of int and Frac.

Related changes

• PR to update pingcap/docs/pingcap/docs-cn:
• Need to cherry-pick to the release branch

Check List

Tests

• Unit test
• Integration test

Side effects

• Performance regression
  • Consumes more CPU
  • Consumes more MEM

Release note

• fix wrong hash key for decimal

[qw4990]

LGTM

[lzmhhh123]

/run-all-tests

[breezewish]

This test case shows that current implementation is still incorrect:

func TestDec(t *testing.T) {
	dec1 := &MyDecimal{}
	if err := dec1.FromString([]byte(".1is column 'ommand den6ser '%-956465")); err != nil {
		panic("dec1")
	}

	dec2 := &MyDecimal{}
	if err := dec2.FromString([]byte("0.10s e8s'@'%-.664000000000000000000000000000")); err != nil {
		panic("dec2")
	}

	if dec1.Compare(dec2) != 0 {
		panic("dec not equal")
	}

	h1, err := dec1.ToHashKey()
	if err != nil {
		panic(err)
	}

	h2, err := dec2.ToHashKey()
	if err != nil {
		panic(err)
	}

	if bytes.Compare(h1, h2) != 0 {
		panic("dec1 eq dec2 but dec1 hash not eq dec2 hash")
	}
}

panic: dec1 eq dec2 but dec1 hash not eq dec2 hash

[lzmhhh123]

This test case shows that current implementation is still incorrect:

func TestDec(t *testing.T) {
	dec1 := &MyDecimal{}
	if err := dec1.FromString([]byte(".1is column 'ommand den6ser '%-956465")); err != nil {
		panic("dec1")
	}

	dec2 := &MyDecimal{}
	if err := dec2.FromString([]byte("0.10s e8s'@'%-.664000000000000000000000000000")); err != nil {
		panic("dec2")
	}

	if dec1.Compare(dec2) != 0 {
		panic("dec not equal")
	}

	h1, err := dec1.ToHashKey()
	if err != nil {
		panic(err)
	}

	h2, err := dec2.ToHashKey()
	if err != nil {
		panic(err)
	}

	if bytes.Compare(h1, h2) != 0 {
		panic("dec1 eq dec2 but dec1 hash not eq dec2 hash")
	}
}

panic: dec1 eq dec2 but dec1 hash not eq dec2 hash

0.1 and 0.10 have different hash key. It's another bug.

[lzmhhh123]

@breeswish @SunRunAway PTAL. Already fixed.

[breezewish]

Here is a case that value eq but hash not eq using the latest code base:

dec1 = 0000000000000000000000000000000000000000000.0000000000005987000000000000000/000000000000000000000000000000000000
dec2 = 00000000000000000000000000000.000000000000598700

[breezewish]

I found some new bugs. "ToHashKey" function will return errors in some cases. I'm not sure whether this case should be handled:

dec1 = 4e-41
dec2 = 4e-41

[lzmhhh123]

I found some new bugs. "ToHashKey" function will return errors in some cases. I'm not sure whether this case should be handled:

dec1 = 4e-41
dec2 = 4e-41

TiDB's MaxDecimalScale is 30.

[breezewish]

Approve since fuzz test doesn't show anything wrong now.

[codecov]

Codecov Report

Merging #19131 into master will decrease coverage by 0.1248%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             master     #19131        +/-   ##
================================================
- Coverage   79.3723%   79.2474%   -0.1249%     
================================================
  Files           549        550         +1     
  Lines        150647     150391       -256     
================================================
- Hits         119572     119181       -391     
- Misses        21601      21680        +79     
- Partials       9474       9530        +56     

[ichn-hu]

This PR could close #17216

Would you please add a test case for this issue? It has a minial test case here #17216 (comment)

[SunRunAway]

LGTM

[SunRunAway]

/merge

[ti-srebot]

/run-all-tests

[ti-srebot]

cherry pick to release-3.0 in PR #19185

[ti-srebot]

cherry pick to release-3.1 in PR #19187

[ti-srebot]

cherry pick to release-4.0 in PR #19188