Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(math): Upstream GDA based decimal type - marshal/unmarshal #21016

Closed
wants to merge 5 commits into from
Closed

feat(math): Upstream GDA based decimal type - marshal/unmarshal #21016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bd4c5cb
marshall/unmarshall
samricotta Jul 22, 2024
15fc0ef
remove tests that are from deleted constraints
samricotta Jul 22, 2024
16f5b25
update godocs and add more tests
samricotta Jul 24, 2024
bf25586
pure decimal changes
samricotta Jul 30, 2024
7ede82f
updates on using scientific notation
samricotta Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions math/dec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,12 +368,27 @@ func (x Dec) Reduce() (Dec, int) {

// Marshal serializes the decimal value into a byte slice in a text format.
// This method ensures the decimal is represented in a portable and human-readable form.
func (d Dec) Marshal() ([]byte, error) {
return d.dec.MarshalText()
// The output may be in scientific notation if the number's magnitude is very large or very small.
//
// Returns:
// - A byte slice of the decimal in text format, which may include scientific notation depending on the value.
func (x Dec) Marshal() ([]byte, error) {
return x.dec.MarshalText()
samricotta marked this conversation as resolved.
Show resolved Hide resolved
}

// Unmarshal parses a byte slice containing a text-formatted decimal and stores the result in the receiver.
// It returns an error if the byte slice does not represent a valid decimal.
func (d *Dec) Unmarshal(data []byte) error {
return d.dec.UnmarshalText(data)
func (x *Dec) Unmarshal(data []byte) error {
result, err := NewDecFromString(string(data))
if err != nil {
return ErrInvalidDec.Wrap(err.Error())
}

if result.dec.Form != apd.Finite {
return ErrInvalidDec.Wrap("unknown decimal form")
}

x.dec = result.dec
return nil

}
57 changes: 0 additions & 57 deletions math/dec_rapid_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"regexp"
"strconv"
"strings"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -18,10 +17,6 @@ func TestDecWithRapid(t *testing.T) {

// Properties about *FromString functions
t.Run("TestInvalidNewDecFromString", rapid.MakeCheck(testInvalidNewDecFromString))
t.Run("TestInvalidNewNonNegativeDecFromString", rapid.MakeCheck(testInvalidNewNonNegativeDecFromString))
t.Run("TestInvalidNewNonNegativeFixedDecFromString", rapid.MakeCheck(testInvalidNewNonNegativeFixedDecFromString))
t.Run("TestInvalidNewPositiveDecFromString", rapid.MakeCheck(testInvalidNewPositiveDecFromString))
t.Run("TestInvalidNewPositiveFixedDecFromString", rapid.MakeCheck(testInvalidNewPositiveFixedDecFromString))

// Properties about addition
t.Run("TestAddLeftIdentity", rapid.MakeCheck(testAddLeftIdentity))
Expand Down Expand Up @@ -191,58 +186,6 @@ func testInvalidNewDecFromString(t *rapid.T) {
require.Error(t, err)
}

// Property: invalid_number_string(s) || IsNegative(s)
// => NewNonNegativeDecFromString(s) == err
func testInvalidNewNonNegativeDecFromString(t *rapid.T) {
s := rapid.OneOf(
rapid.StringMatching("[[:alpha:]]+"),
rapid.StringMatching(`^-\d*\.?\d+$`).Filter(
func(s string) bool { return !strings.HasPrefix(s, "-0") && !strings.HasPrefix(s, "-.0") },
),
).Draw(t, "s")
_, err := NewDecFromString(s)
require.Error(t, err)
}

// Property: invalid_number_string(s) || IsNegative(s) || NumDecimals(s) > n
// => NewNonNegativeFixedDecFromString(s, n) == err
func testInvalidNewNonNegativeFixedDecFromString(t *rapid.T) {
n := rapid.Uint32Range(0, 999).Draw(t, "n")
s := rapid.OneOf(
rapid.StringMatching("[[:alpha:]]+"),
rapid.StringMatching(`^-\d*\.?\d+$`).Filter(
func(s string) bool { return !strings.HasPrefix(s, "-0") && !strings.HasPrefix(s, "-.0") },
),
rapid.StringMatching(fmt.Sprintf(`\d*\.\d{%d,}`, n+1)),
).Draw(t, "s")
_, err := NewDecFromString(s)
require.Error(t, err)
}

// Property: invalid_number_string(s) || IsNegative(s) || IsZero(s)
// => NewPositiveDecFromString(s) == err
func testInvalidNewPositiveDecFromString(t *rapid.T) {
s := rapid.OneOf(
rapid.StringMatching("[[:alpha:]]+"),
rapid.StringMatching(`^-\d*\.?\d+|0$`),
).Draw(t, "s")
_, err := NewDecFromString(s)
require.Error(t, err)
}

// Property: invalid_number_string(s) || IsNegative(s) || IsZero(s) || NumDecimals(s) > n
// => NewPositiveFixedDecFromString(s) == err
func testInvalidNewPositiveFixedDecFromString(t *rapid.T) {
n := rapid.Uint32Range(0, 999).Draw(t, "n")
s := rapid.OneOf(
rapid.StringMatching("[[:alpha:]]+"),
rapid.StringMatching(`^-\d*\.?\d+|0$`),
rapid.StringMatching(fmt.Sprintf(`\d*\.\d{%d,}`, n+1)),
).Draw(t, "s")
_, err := NewDecFromString(s)
require.Error(t, err)
}

// Property: 0 + a == a
func testAddLeftIdentity(t *rapid.T) {
a := genDec.Draw(t, "a")
Expand Down
185 changes: 185 additions & 0 deletions math/dec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strings"
"testing"

"github.com/cockroachdb/apd/v3"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -1287,3 +1288,187 @@ func must[T any](r T, err error) T {
}
return r
}

func TestMarshal(t *testing.T) {
specs := map[string]struct {
x Dec
exp string
}{
"No trailing zeros": {
x: NewDecFromInt64(123456),
exp: "123456",
},
"Trailing zeros": {
x: NewDecFromInt64(123456000),
exp: "123456000",
},
"Zero value": {
x: NewDecFromInt64(0),
exp: "0",
},
"Decimal value": {
x: must(NewDecFromString("1.30000")),
exp: "1.30000",
},
"Positive value": {
x: NewDecFromInt64(10),
exp: "10",
},
"negative value": {
x: NewDecFromInt64(-10),
exp: "-10",
},
"max decimal": {
x: must(NewDecFromString("9." + strings.Repeat("0", 34))),
exp: "9.0000000000000000000000000000000000",
},
"min decimal": {
x: must(NewDecFromString("-1." + strings.Repeat("0", 34))),
exp: "-1.0000000000000000000000000000000000",
},
"6 decimal places": {
x: must(NewDecFromString("0.000001")),
exp: "0.000001",
},
"7 decimal places": {
x: must(NewDecFromString("0.0000001")),
exp: "1E-7",
},
"1e100000": {
x: NewDecWithPrec(1, 100_000),
exp: "1E+100000",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here and others: I was expecting `"100000000000..." not the scientific notation.

},
"1.1e100000": {
x: NewDecWithPrec(11, 100_000),
exp: "1.1E+100001",
},
"1.e100000": {
x: NewDecWithPrec(1, 100_000),
exp: "1E+100000",
},
}
for name, spec := range specs {
t.Run(name, func(t *testing.T) {
marshaled, err := spec.x.Marshal()
require.NoError(t, err)
assert.Equal(t, spec.exp, string(marshaled))
})
}
}

func TestUnMarshal(t *testing.T) {
specs := map[string]struct {
x string
exp string
expErr error
}{
"Leading zeros": {
x: "000123456",
exp: "123456",
},
"Trailing zeros": {
x: "1.00000",
exp: "1.00000",
},
"Small e": {
x: "1.23456e+8",
exp: "123456000",
},
"Zero value": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to include -0 as an edge case in the test spec

x: "0",
exp: "0",
},
"-0": {
x: "-0",
exp: "-0",
},
"Decimal value": {
x: "1.3000",
exp: "1.3000",
},
"Positive value": {
x: "1E+1",
exp: "10",
},
"negative value": {
x: "-1E+1",
exp: "-10",
},
"Max Exponent": {
x: "1e" + strconv.Itoa(apd.MaxExponent),
exp: "1e" + strconv.Itoa(apd.MaxExponent),
},
"Above Max Exponent": {
x: "1e" + strconv.Itoa(apd.MaxExponent+1),
expErr: ErrInvalidDec,
},
"Min Exponent": {
x: "1e-100000",
exp: "1e-100000",
},
"Below Min Exponent": {
x: "1e" + strconv.Itoa(apd.MinExponent-1),
expErr: ErrInvalidDec,
},
"1e100000": {
x: "1E+100000",
exp: "1e100000",
},
"1.1e100000": {
x: "1.1E+100001",
expErr: ErrInvalidDec,
},
"1.e100000": {
x: "1E+100000",
exp: "1e100000",
},
"-1e100000": {
x: "-1e100000",
exp: "-1e100000",
},
"9e100000": {
x: "9e100000",
exp: "9e100000",
},
"NaN": {
x: "NaN",
expErr: ErrInvalidDec,
},
"1foo": {
x: "1foo",
expErr: ErrInvalidDec,
alpe marked this conversation as resolved.
Show resolved Hide resolved
},
".": {
x: ".",
expErr: ErrInvalidDec,
},
"0.": {
x: "0.",
exp: "0",
},
".0": {
x: ".0",
exp: "0.0",
},
}
for name, spec := range specs {
t.Run(name, func(t *testing.T) {
var unmarshaled Dec
err := unmarshaled.Unmarshal([]byte(spec.x))
if spec.expErr != nil {
require.ErrorIs(t, err, spec.expErr)
return
}
if unmarshaled.dec.Exponent == 100000 || unmarshaled.dec.Exponent == -100000 {
if unmarshaled.dec.Negative {
assert.Equal(t, spec.exp, "-"+unmarshaled.dec.Coeff.String()+"e"+strconv.Itoa(int(unmarshaled.dec.Exponent)))
} else {
assert.Equal(t, spec.exp, unmarshaled.dec.Coeff.String()+"e"+strconv.Itoa(int(unmarshaled.dec.Exponent)))
}
} else {
assert.Equal(t, spec.exp, unmarshaled.String())
}
require.NoError(t, err)
})
}
}
Loading