Go's decimal implementation cannot represent all Ion decimals with fidelity

[wesboyt]

Decimals(and thus timestamps) in GO do not conform to the Ion Specification and will corrupt data once write functionality is implemented.
As such non equivs test vectors should not be passing.

[mrutter-amzn]

Can you be more explicit? Go doesn't natively have support for Decimals, it has support for floats. How will data be corrupted when write functionality is implemented?

[wesboyt]

Timestamps with a fractional component and decimals will be altered roundtrip. Dropping precision or rounding the value at all is not spec compliant.

[mrutter-amzn]

The Timestamp type handle the fractional component independently of the Decimal type. Timestamp retains the precision indicator.

In my view this issue should specifically be about the Decimal type and how the Decimal type does not track precision. The general title and description are misleading.

[wesboyt]

Sorry I wasn't very clear, What we need is our own decimal implementation that can create go native decimals when users ask for them. Decimals cant lose precision or alter data when passed from a reader to a writer.

[almann]

I decided to look a bit closer at what we're doing. Firstly, we are using shopspring/decimal, so
specifically, the representation of a Decimal is as follows:

decimal.go#L69-L80

// Decimal represents a fixed-point decimal. It is immutable.
// number = value * 10 ^ exp
type Decimal struct {
	value *big.Int

	// NOTE(vadim): this must be an int32, because we cast it to float64 during
	// calculations. If exp is 64 bit, we might lose precision.
	// If we cared about being able to represent every possible decimal, we
	// could make exp a *big.Int but it would hurt performance and numbers
	// like that are unrealistic.
	exp int32
}

This is important because, Decimal is immutable and we have accessors to get the fields in question:

decimal.go#L684-L695

// Exponent returns the exponent, or scale component of the decimal.
func (d Decimal) Exponent() int32 {
	return d.exp
}


// Coefficient returns the coefficient of the decimal.  It is scaled by 10^Exponent()
func (d Decimal) Coefficient() *big.Int {
	d.ensureInitialized()
	// we copy the coefficient so that mutating the result does not mutate the
	// Decimal.
	return big.NewInt(0).Set(d.value)
}

Importantly, the parsing code of the library we use does strip out trailing zeros (see: shopspring/decimal#107 and shopspring/decimal#31), but the representation is sound, we can technically work around the serialization/deserialization problems around precision ourselves without implementing a whole decimal library. It should be noted, that the library's author is inclined to fix this issue so we may not need to work around it when they get around to supporting what we need here.

The other problem is that per shopspring/decimal#150, the library does not support negative zero. This is not a fatal problem as we can encapsulate that within the Value API we expose (similarly, Java's BigDecimal doesn't support negative zero, and we work around it there too). The author does not seem inclined to fix this issue, but as long as we have a way to preserve and represent negative zero in our APIs, we can still utilize this library for our decimal representation.