From 5f76bc637461b06b229956c53321d90ad30c0e9a Mon Sep 17 00:00:00 2001 From: Oleg Jukovec Date: Tue, 20 Jun 2023 11:22:57 +0300 Subject: [PATCH] api: the Decimal type is immutable The patch forces the use of objects of type Decimal instead of pointers. Part of #238 --- CHANGELOG.md | 1 + README.md | 6 +++++ decimal/decimal.go | 58 ++++++++++++++++++++++++++--------------- decimal/decimal_test.go | 54 ++++++++++++++++++++------------------ decimal/example_test.go | 2 +- 5 files changed, 73 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fcd90d53..5ece0ade0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release. - connection_pool renamed to pool (#239) - Use msgpack/v5 instead of msgpack.v2 (#236) - Call/NewCallRequest = Call17/NewCall17Request (#235) +- Use objects of the Decimal type instead of pointers (#238) ### Deprecated diff --git a/README.md b/README.md index b3bff4d3d..df71d2ad9 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,7 @@ faster than other packages according to public benchmarks. * [API reference](#api-reference) * [Walking\-through example](#walking-through-example) * [Migration to v2](#migration-to-v2) + * [decimal package](#decimal-package) * [multi package](#multi-package) * [pool package](#pool-package) * [msgpack.v5](#msgpackv5) @@ -148,6 +149,11 @@ by `Connect()`. The article describes migration from go-tarantool to go-tarantool/v2. +#### decimal package + +Now you need to use objects of the Decimal type instead of pointers to it. A +new constructor `MakeDecimal` returns an object. `NewDecimal` has been removed. + #### multi package The subpackage has been deleted. You could use `pool` instead. diff --git a/decimal/decimal.go b/decimal/decimal.go index 6cca53404..31e036a22 100644 --- a/decimal/decimal.go +++ b/decimal/decimal.go @@ -17,6 +17,7 @@ package decimal import ( "fmt" + "reflect" "github.com/shopspring/decimal" "github.com/vmihailenco/msgpack/v5" @@ -37,38 +38,44 @@ const ( decimalPrecision = 38 ) +var ( + one decimal.Decimal = decimal.NewFromInt(1) + // -10^decimalPrecision - 1 + minSupportedDecimal decimal.Decimal = maxSupportedDecimal.Neg().Sub(one) + // 10^decimalPrecision - 1 + maxSupportedDecimal decimal.Decimal = decimal.New(1, decimalPrecision).Sub(one) +) + type Decimal struct { decimal.Decimal } -// NewDecimal creates a new Decimal from a decimal.Decimal. -func NewDecimal(decimal decimal.Decimal) *Decimal { - return &Decimal{Decimal: decimal} +// MakeDecimal creates a new Decimal from a decimal.Decimal. +func MakeDecimal(decimal decimal.Decimal) Decimal { + return Decimal{Decimal: decimal} } -// NewDecimalFromString creates a new Decimal from a string. -func NewDecimalFromString(src string) (result *Decimal, err error) { +// MakeDecimalFromString creates a new Decimal from a string. +func MakeDecimalFromString(src string) (Decimal, error) { + result := Decimal{} dec, err := decimal.NewFromString(src) if err != nil { - return + return result, err } - result = NewDecimal(dec) - return + result = MakeDecimal(dec) + return result, nil } -// MarshalMsgpack serializes the Decimal into a MessagePack representation. -func (decNum *Decimal) MarshalMsgpack() ([]byte, error) { - one := decimal.NewFromInt(1) - maxSupportedDecimal := decimal.New(1, decimalPrecision).Sub(one) // 10^decimalPrecision - 1 - minSupportedDecimal := maxSupportedDecimal.Neg().Sub(one) // -10^decimalPrecision - 1 - if decNum.GreaterThan(maxSupportedDecimal) { +func decimalEncoder(e *msgpack.Encoder, v reflect.Value) ([]byte, error) { + dec := v.Interface().(Decimal) + if dec.GreaterThan(maxSupportedDecimal) { return nil, fmt.Errorf("msgpack: decimal number is bigger than maximum supported number (10^%d - 1)", decimalPrecision) } - if decNum.LessThan(minSupportedDecimal) { + if dec.LessThan(minSupportedDecimal) { return nil, fmt.Errorf("msgpack: decimal number is lesser than minimum supported number (-10^%d - 1)", decimalPrecision) } - strBuf := decNum.String() + strBuf := dec.String() bcdBuf, err := encodeStringToBCD(strBuf) if err != nil { return nil, fmt.Errorf("msgpack: can't encode string (%s) to a BCD buffer: %w", strBuf, err) @@ -76,9 +83,16 @@ func (decNum *Decimal) MarshalMsgpack() ([]byte, error) { return bcdBuf, nil } -// UnmarshalMsgpack deserializes a Decimal value from a MessagePack -// representation. -func (decNum *Decimal) UnmarshalMsgpack(b []byte) error { +func decimalDecoder(d *msgpack.Decoder, v reflect.Value, extLen int) error { + b := make([]byte, extLen) + n, err := d.Buffered().Read(b) + if err != nil { + return err + } + if n < extLen { + return fmt.Errorf("msgpack: unexpected end of stream after %d decimal bytes", n) + } + // Decimal values can be encoded to fixext MessagePack, where buffer // has a fixed length encoded by first byte, and ext MessagePack, where // buffer length is not fixed and encoded by a number in a separate @@ -92,14 +106,16 @@ func (decNum *Decimal) UnmarshalMsgpack(b []byte) error { 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) } + ptr := v.Addr().Interface().(*Decimal) + *ptr = MakeDecimal(dec) return nil } func init() { - msgpack.RegisterExt(decimalExtID, (*Decimal)(nil)) + msgpack.RegisterExtDecoder(decimalExtID, Decimal{}, decimalDecoder) + msgpack.RegisterExtEncoder(decimalExtID, Decimal{}, decimalEncoder) } diff --git a/decimal/decimal_test.go b/decimal/decimal_test.go index e3eb867c2..fe806340d 100644 --- a/decimal/decimal_test.go +++ b/decimal/decimal_test.go @@ -63,10 +63,10 @@ func (t *TupleDecimal) DecodeMsgpack(d *msgpack.Decoder) error { return err } - if dec, ok := res.(*Decimal); !ok { + if dec, ok := res.(Decimal); !ok { return fmt.Errorf("decimal doesn't match") } else { - t.number = *dec + t.number = dec } return nil } @@ -143,12 +143,12 @@ var decimalSamples = []struct { func TestMPEncodeDecode(t *testing.T) { for _, testcase := range benchmarkSamples { t.Run(testcase.numString, func(t *testing.T) { - decNum, err := NewDecimalFromString(testcase.numString) + decNum, err := MakeDecimalFromString(testcase.numString) if err != nil { t.Fatal(err) } var buf []byte - tuple := TupleDecimal{number: *decNum} + tuple := TupleDecimal{number: decNum} if buf, err = msgpack.Marshal(&tuple); err != nil { t.Fatalf("Failed to msgpack.Encoder decimal number '%s' to a MessagePack buffer: %s", testcase.numString, err) } @@ -251,7 +251,7 @@ func TestEncodeStringToBCDIncorrectNumber(t *testing.T) { func TestEncodeMaxNumber(t *testing.T) { referenceErrMsg := "msgpack: decimal number is bigger than maximum supported number (10^38 - 1)" decNum := decimal.New(1, DecimalPrecision) // // 10^DecimalPrecision - tuple := TupleDecimal{number: *NewDecimal(decNum)} + tuple := TupleDecimal{number: MakeDecimal(decNum)} _, err := msgpack.Marshal(&tuple) if err == nil { t.Fatalf("It is possible to msgpack.Encoder a number unsupported by Tarantool") @@ -265,7 +265,7 @@ func TestEncodeMinNumber(t *testing.T) { referenceErrMsg := "msgpack: decimal number is lesser than minimum supported number (-10^38 - 1)" two := decimal.NewFromInt(2) decNum := decimal.New(1, DecimalPrecision).Neg().Sub(two) // -10^DecimalPrecision - 2 - tuple := TupleDecimal{number: *NewDecimal(decNum)} + tuple := TupleDecimal{number: MakeDecimal(decNum)} _, err := msgpack.Marshal(&tuple) if err == nil { t.Fatalf("It is possible to msgpack.Encoder a number unsupported by Tarantool") @@ -284,7 +284,7 @@ func benchmarkMPEncodeDecode(b *testing.B, src decimal.Decimal, dst interface{}) var buf []byte var err error for i := 0; i < b.N; i++ { - tuple := TupleDecimal{number: *NewDecimal(src)} + tuple := TupleDecimal{number: MakeDecimal(src)} if buf, err = msgpack.Marshal(&tuple); err != nil { b.Fatal(err) } @@ -309,13 +309,15 @@ func BenchmarkMPEncodeDecodeDecimal(b *testing.B) { func BenchmarkMPEncodeDecimal(b *testing.B) { for _, testcase := range benchmarkSamples { b.Run(testcase.numString, func(b *testing.B) { - decNum, err := NewDecimalFromString(testcase.numString) + decNum, err := MakeDecimalFromString(testcase.numString) if err != nil { b.Fatal(err) } b.ResetTimer() for i := 0; i < b.N; i++ { - msgpack.Marshal(decNum) + if _, err := msgpack.Marshal(decNum); err != nil { + b.Fatal(err) + } } }) } @@ -324,20 +326,20 @@ func BenchmarkMPEncodeDecimal(b *testing.B) { func BenchmarkMPDecodeDecimal(b *testing.B) { for _, testcase := range benchmarkSamples { b.Run(testcase.numString, func(b *testing.B) { - decNum, err := NewDecimalFromString(testcase.numString) + decNum, err := MakeDecimalFromString(testcase.numString) if err != nil { b.Fatal(err) } - var buf []byte - if buf, err = msgpack.Marshal(decNum); err != nil { + buf, err := msgpack.Marshal(decNum) + if err != nil { b.Fatal(err) } b.ResetTimer() - var v TupleDecimal for i := 0; i < b.N; i++ { - msgpack.Unmarshal(buf, &v) + if err := msgpack.Unmarshal(buf, &decNum); err != nil { + b.Fatal(err) + } } - }) } } @@ -353,7 +355,7 @@ func tupleValueIsDecimal(t *testing.T, tuples []interface{}, number decimal.Deci if len(tpl) != 1 { t.Fatalf("Unexpected return value body (tuple len)") } - if val, ok := tpl[0].(*Decimal); !ok || !val.Equal(number) { + if val, ok := tpl[0].(Decimal); !ok || !val.Equal(number) { t.Fatalf("Unexpected return value body (tuple 0 field)") } } @@ -418,9 +420,9 @@ func TestMPEncode(t *testing.T) { samples = append(samples, benchmarkSamples...) for _, testcase := range samples { t.Run(testcase.numString, func(t *testing.T) { - dec, err := NewDecimalFromString(testcase.numString) + dec, err := MakeDecimalFromString(testcase.numString) if err != nil { - t.Fatalf("NewDecimalFromString() failed: %s", err.Error()) + t.Fatalf("MakeDecimalFromString() failed: %s", err.Error()) } buf, err := msgpack.Marshal(dec) if err != nil { @@ -451,7 +453,7 @@ func TestMPDecode(t *testing.T) { if err != nil { t.Fatalf("Unmsgpack.Marshalling failed: %s", err.Error()) } - decActual, ok := v.(*Decimal) + decActual, ok := v.(Decimal) if !ok { t.Fatalf("Unable to convert to Decimal") } @@ -502,7 +504,7 @@ func TestSelect(t *testing.T) { t.Fatalf("Failed to prepare test decimal: %s", err) } - ins := NewInsertRequest(space).Tuple([]interface{}{NewDecimal(number)}) + ins := NewInsertRequest(space).Tuple([]interface{}{MakeDecimal(number)}) resp, err := conn.Do(ins).Get() if err != nil { t.Fatalf("Decimal insert failed: %s", err) @@ -519,7 +521,7 @@ func TestSelect(t *testing.T) { Offset(offset). Limit(limit). Iterator(IterEq). - Key([]interface{}{NewDecimal(number)}) + Key([]interface{}{MakeDecimal(number)}) resp, err = conn.Do(sel).Get() if err != nil { t.Fatalf("Decimal select failed: %s", err.Error()) @@ -529,7 +531,7 @@ func TestSelect(t *testing.T) { } tupleValueIsDecimal(t, resp.Data, number) - del := NewDeleteRequest(space).Index(index).Key([]interface{}{NewDecimal(number)}) + del := NewDeleteRequest(space).Index(index).Key([]interface{}{MakeDecimal(number)}) resp, err = conn.Do(del).Get() if err != nil { t.Fatalf("Decimal delete failed: %s", err) @@ -543,7 +545,7 @@ func assertInsert(t *testing.T, conn *Connection, numString string) { t.Fatalf("Failed to prepare test decimal: %s", err) } - ins := NewInsertRequest(space).Tuple([]interface{}{NewDecimal(number)}) + ins := NewInsertRequest(space).Tuple([]interface{}{MakeDecimal(number)}) resp, err := conn.Do(ins).Get() if err != nil { t.Fatalf("Decimal insert failed: %s", err) @@ -553,7 +555,7 @@ func assertInsert(t *testing.T, conn *Connection, numString string) { } tupleValueIsDecimal(t, resp.Data, number) - del := NewDeleteRequest(space).Index(index).Key([]interface{}{NewDecimal(number)}) + del := NewDeleteRequest(space).Index(index).Key([]interface{}{MakeDecimal(number)}) resp, err = conn.Do(del).Get() if err != nil { t.Fatalf("Decimal delete failed: %s", err) @@ -586,7 +588,7 @@ func TestReplace(t *testing.T) { t.Fatalf("Failed to prepare test decimal: %s", err) } - rep := NewReplaceRequest(space).Tuple([]interface{}{NewDecimal(number)}) + rep := NewReplaceRequest(space).Tuple([]interface{}{MakeDecimal(number)}) respRep, errRep := conn.Do(rep).Get() if errRep != nil { t.Fatalf("Decimal replace failed: %s", errRep) @@ -600,7 +602,7 @@ func TestReplace(t *testing.T) { Index(index). Limit(1). Iterator(IterEq). - Key([]interface{}{NewDecimal(number)}) + Key([]interface{}{MakeDecimal(number)}) respSel, errSel := conn.Do(sel).Get() if errSel != nil { t.Fatalf("Decimal select failed: %s", errSel) diff --git a/decimal/example_test.go b/decimal/example_test.go index c57cc0603..a355767f1 100644 --- a/decimal/example_test.go +++ b/decimal/example_test.go @@ -35,7 +35,7 @@ func Example() { spaceNo := uint32(524) - number, err := NewDecimalFromString("-22.804") + number, err := MakeDecimalFromString("-22.804") if err != nil { log.Fatalf("Failed to prepare test decimal: %s", err) }