Skip to content

Commit

Permalink
format: do not output 0's after decimals for coeff < -2000
Browse files Browse the repository at this point in the history
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
ffd11b1 to only output trailing
zeroes up to -2000.
  • Loading branch information
otan committed Sep 13, 2023
1 parent e746f59 commit fce19ea
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 17 deletions.
13 changes: 13 additions & 0 deletions decimal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"math"
"math/bits"
"strings"
"testing"
"unsafe"
)
Expand Down Expand Up @@ -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"}

Expand Down
34 changes: 21 additions & 13 deletions format.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,13 @@ func (d *Decimal) String() string {
return d.Text('G')
}

// lowestZeroNegativeCoefficientCockroach is the smallest co-efficient in
// Cockroach supports using 0E<coefficient>.
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, '-')
Expand All @@ -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-<exponent> 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
Expand All @@ -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
Expand Down
10 changes: 6 additions & 4 deletions gda_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit fce19ea

Please sign in to comment.