diff --git a/CHANGELOG.md b/CHANGELOG.md index 23c0bb63f..3e53ea2c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release. ### Fixed - Flaky decimal/TestSelect (#300) +- Incorrect decoding of an MP_DECIMAL when the `scale` value is negative (#314) ## [1.12.0] - 2023-06-07 diff --git a/decimal/bcd.go b/decimal/bcd.go index 2b5a4b258..fa3475595 100644 --- a/decimal/bcd.go +++ b/decimal/bcd.go @@ -183,10 +183,10 @@ func encodeStringToBCD(buf string) ([]byte, error) { // ended by a 4-bit sign nibble in the least significant four bits of the final // byte. The scale is used (negated) as the exponent of the decimal number. // Note that zeroes may have a sign and/or a scale. -func decodeStringFromBCD(bcdBuf []byte) (string, error) { +func decodeStringFromBCD(bcdBuf []byte) (string, int, error) { // Index of a byte with scale. const scaleIdx = 0 - scale := int(bcdBuf[scaleIdx]) + scale := int(int8(bcdBuf[scaleIdx])) // Get a BCD buffer without scale. bcdBuf = bcdBuf[scaleIdx+1:] @@ -204,10 +204,6 @@ func decodeStringFromBCD(bcdBuf []byte) (string, error) { // Reserve bytes for dot and sign. numLen := ndigits + 2 - // Reserve bytes for zeroes. - if scale >= ndigits { - numLen += scale - ndigits - } var bld strings.Builder bld.Grow(numLen) @@ -219,26 +215,10 @@ func decodeStringFromBCD(bcdBuf []byte) (string, error) { bld.WriteByte('-') } - // Add missing zeroes to the left side when scale is bigger than a - // number of digits and a single missed zero to the right side when - // equal. - if scale > ndigits { - bld.WriteByte('0') - bld.WriteByte('.') - for diff := scale - ndigits; diff > 0; diff-- { - bld.WriteByte('0') - } - } else if scale == ndigits { - bld.WriteByte('0') - } - const MaxDigit = 0x09 // Builds a buffer with symbols of decimal number (digits, dot and sign). processNibble := func(nibble byte) { if nibble <= MaxDigit { - if ndigits == scale { - bld.WriteByte('.') - } bld.WriteByte(nibble + '0') ndigits-- } @@ -254,5 +234,8 @@ func decodeStringFromBCD(bcdBuf []byte) (string, error) { processNibble(lowNibble) } - return bld.String(), nil + if bld.Len() == 0 { + bld.WriteByte('0') + } + return bld.String(), -1 * scale, nil } diff --git a/decimal/decimal.go b/decimal/decimal.go index 6cca53404..05897a7d7 100644 --- a/decimal/decimal.go +++ b/decimal/decimal.go @@ -87,16 +87,20 @@ func (decNum *Decimal) UnmarshalMsgpack(b []byte) error { // +--------+-------------------+------------+===============+ // | MP_EXT | length (optional) | MP_DECIMAL | PackedDecimal | // +--------+-------------------+------------+===============+ - digits, err := decodeStringFromBCD(b) + digits, exp, err := decodeStringFromBCD(b) if err != nil { return fmt.Errorf("msgpack: can't decode string from BCD buffer (%x): %w", b, err) } + dec, err := decimal.NewFromString(digits) - *decNum = *NewDecimal(dec) if err != nil { return fmt.Errorf("msgpack: can't encode string (%s) to a decimal number: %w", digits, err) } + if exp != 0 { + dec = dec.Shift(int32(exp)) + } + *decNum = *NewDecimal(dec) return nil } diff --git a/decimal/decimal_test.go b/decimal/decimal_test.go index e3eb867c2..47eea3ae5 100644 --- a/decimal/decimal_test.go +++ b/decimal/decimal_test.go @@ -116,6 +116,17 @@ var correctnessSamples = []struct { {"-1234567891234567890.0987654321987654321", "c7150113012345678912345678900987654321987654321d", false}, } +var correctnessDecodeSamples = []struct { + numString string + mpBuf string + fixExt bool +}{ + {"1e2", "d501fe1c", true}, + {"1.1e31", "c70301e2011c", false}, + {"13e-2", "c7030102013c", false}, + {"-1e3", "d501fd1d", true}, +} + // There is a difference between encoding result from a raw string and from // decimal.Decimal. It's expected because decimal.Decimal simplifies decimals: // 0.00010000 -> 0.0001 @@ -387,18 +398,23 @@ func TestEncodeStringToBCD(t *testing.T) { } func TestDecodeStringFromBCD(t *testing.T) { - samples := append(correctnessSamples, rawSamples...) + samples := correctnessSamples + samples = append(samples, correctnessDecodeSamples...) + samples = append(samples, rawSamples...) samples = append(samples, benchmarkSamples...) for _, testcase := range samples { t.Run(testcase.numString, func(t *testing.T) { b, _ := hex.DecodeString(testcase.mpBuf) bcdBuf := trimMPHeader(b, testcase.fixExt) - s, err := DecodeStringFromBCD(bcdBuf) + s, exp, err := DecodeStringFromBCD(bcdBuf) if err != nil { t.Fatalf("Failed to decode BCD '%x' to decimal: %s", bcdBuf, err) } decActual, err := decimal.NewFromString(s) + if exp != 0 { + decActual = decActual.Shift(int32(exp)) + } if err != nil { t.Fatalf("Failed to msgpack.Encoder string ('%s') to decimal", s) } diff --git a/decimal/export_test.go b/decimal/export_test.go index c43a812c6..2c8fda1c7 100644 --- a/decimal/export_test.go +++ b/decimal/export_test.go @@ -4,7 +4,7 @@ func EncodeStringToBCD(buf string) ([]byte, error) { return encodeStringToBCD(buf) } -func DecodeStringFromBCD(bcdBuf []byte) (string, error) { +func DecodeStringFromBCD(bcdBuf []byte) (string, int, error) { return decodeStringFromBCD(bcdBuf) } diff --git a/decimal/fuzzing_test.go b/decimal/fuzzing_test.go index c7df2e080..5ec2a2b8f 100644 --- a/decimal/fuzzing_test.go +++ b/decimal/fuzzing_test.go @@ -10,11 +10,14 @@ import ( . "github.com/tarantool/go-tarantool/v2/decimal" ) -func strToDecimal(t *testing.T, buf string) decimal.Decimal { +func strToDecimal(t *testing.T, buf string, exp int) decimal.Decimal { decNum, err := decimal.NewFromString(buf) if err != nil { t.Fatal(err) } + if exp != 0 { + decNum = decNum.Shift(int32(exp)) + } return decNum } @@ -33,13 +36,13 @@ func FuzzEncodeDecodeBCD(f *testing.F) { if err != nil { t.Skip("Only correct requests are interesting: %w", err) } - var dec string - dec, err = DecodeStringFromBCD(bcdBuf) + + dec, exp, err := DecodeStringFromBCD(bcdBuf) if err != nil { t.Fatalf("Failed to decode encoded value ('%s')", orig) } - if !strToDecimal(t, dec).Equal(strToDecimal(t, orig)) { + if !strToDecimal(t, dec, exp).Equal(strToDecimal(t, orig, 0)) { t.Fatal("Decimal numbers are not equal") } })