Merge pull request #130 from otan-cockroach/coeff_fix

format: do not output 0's after decimals for coeff < -2000

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"}
 

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<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, '-')
@@ -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
@@ -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

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)