From ba96220cee08690953b1f2cf5156dca5d6f8bc19 Mon Sep 17 00:00:00 2001 From: Oliver Tan Date: Wed, 13 Sep 2023 10:21:10 +1000 Subject: [PATCH] format: do not output 0's after decimals for coeff <= -2000 In Cockroach, the minimum supported coefficient for 0 is -2000. To avoid the memory overload that occurs when we try format a large 0 coefficient like `0E-10000`, we amend the fix made in ffd11b1d1ea290d5ebedb620e2a59e8b62897fb2 to only output trailing zeroes up to -2000. --- decimal_test.go | 13 +++++++++++++ format.go | 34 +++++++++++++++++++++------------- gda_test.go | 10 ++++++---- 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/decimal_test.go b/decimal_test.go index 281b84f..ef6a475 100644 --- a/decimal_test.go +++ b/decimal_test.go @@ -19,6 +19,7 @@ import ( "fmt" "math" "math/bits" + "strings" "testing" "unsafe" ) @@ -453,6 +454,18 @@ func TestFormat(t *testing.T) { g: "0.000000000", G: "0.000000000", }, + "0E-2000": { + e: "0e-2000", + f: "0." + strings.Repeat("0", 2000), + g: "0." + strings.Repeat("0", 2000), + G: "0." + strings.Repeat("0", 2000), + }, + "0E-2001": { + e: "0e-2001", + f: "0." + strings.Repeat("0", 2001), + g: "0e-2001", + G: "0E-2001", + }, } verbs := []string{"%e", "%E", "%f", "%g", "%G"} diff --git a/format.go b/format.go index c40fc47..95dfa05 100644 --- a/format.go +++ b/format.go @@ -36,9 +36,13 @@ func (d *Decimal) String() string { return d.Text('G') } +// lowestZeroNegativeCoefficientCockroach is the smallest co-efficient in +// Cockroach supports using 0E. +const lowestZeroNegativeCoefficientCockroach = -2000 + // Append appends to buf the string form of the decimal number d, // as generated by d.Text, and returns the extended buffer. -func (d *Decimal) Append(buf []byte, fmt byte) []byte { +func (d *Decimal) Append(buf []byte, fmtString byte) []byte { // sign if d.Negative { buf = append(buf, '-') @@ -59,20 +63,24 @@ func (d *Decimal) Append(buf []byte, fmt byte) []byte { var scratch [16]byte digits := d.Coeff.Append(scratch[:0], 10) - switch fmt { + switch fmtString { case 'e', 'E': - return fmtE(buf, fmt, d, digits) + return fmtE(buf, fmtString, d, digits) case 'f': return fmtF(buf, d, digits) case 'g', 'G': - // Ensure that we correctly display all 0s like PostgreSQL does. - // For 0.00000000, the co-efficient is represented as 0 - // and hence the digits is incorrectly 0 (for 1.000000, it'd be - // 10000000). Hence, pad the digit length before calculating the - // adjExponentLimit. - // See: https://github.com/cockroachdb/cockroach/issues/102217. digitLen := len(digits) - if d.Coeff.BitLen() == 0 && d.Exponent < 0 { + // PG formats all 0s after the decimal point in the 0E- case + // (e.g. 0E-9 should be 0.000000000). With the `adjExponentLimit` below, + // this does not do that, so for 0 with negative coefficients we pad + // the digit length. + // Ref: https://github.com/cockroachdb/cockroach/issues/102217 + // + // To avoid leaking too much memory for pathological cases, e.g. + // 0E-100000000, we also fall back to the default exponent format + // handling when the exponent is below cockroach's lowest supported 0 + // coefficient. + if d.Coeff.BitLen() == 0 && d.Exponent >= lowestZeroNegativeCoefficientCockroach && d.Exponent < 0 { digitLen += int(-d.Exponent) } // See: http://speleotrove.com/decimal/daconvs.html#reftostr @@ -82,15 +90,15 @@ func (d *Decimal) Append(buf []byte, fmt byte) []byte { return fmtF(buf, d, digits) } // We need to convert the either g or G into a e or E since that's what fmtE - // expects. This is indeed fmt - 2, but attempting to do that in a way that + // expects. This is indeed fmtString - 2, but attempting to do that in a way that // illustrates the intention. - return fmtE(buf, fmt+'e'-'g', d, digits) + return fmtE(buf, fmtString+'e'-'g', d, digits) } if d.Negative { buf = buf[:len(buf)-1] // sign was added prematurely - remove it again } - return append(buf, '%', fmt) + return append(buf, '%', fmtString) } // %e: d.ddddde±d diff --git a/gda_test.go b/gda_test.go index 392f437..8bd366e 100644 --- a/gda_test.go +++ b/gda_test.go @@ -695,11 +695,13 @@ func gdaTest(t *testing.T, path string, tcs []TestCase) { if err != nil { t.Fatalf("unexpected error converting int: %v", err) } - expected = "" - if neg { - expected = "-" + if p <= -lowestZeroNegativeCoefficientCockroach { + expected = "" + if neg { + expected = "-" + } + expected += "0." + strings.Repeat("0", int(p)) } - expected += "0." + strings.Repeat("0", int(p)) } if !strings.EqualFold(s, expected) { t.Fatalf("expected %s, got %s", expected, s)