sql: 0 is formatted incorrectly for DECIMALs with scale >= 7

[otan]

In CockroachDB, 0 is formatted differently for DECIMAL(19,9):

root@localhost:26257/defaultdb> select 0::decimal(19,9);
  numeric
-----------
     0E-9
(1 row)

Compared to postgresql:

otan=# select '0E-9'::decimal(19,9);
   numeric
-------------
 0.000000000
(1 row)

This seems to happen for scale >= 7:

demo@127.0.0.1:26257/defaultdb> select 0::decimal(19,5), 0::decimal(19,6), 0::decimal(19,7);
  numeric | numeric  | numeric
----------+----------+----------
  0.00000 | 0.000000 |    0E-7
(1 row)

Time: 1ms total (execution 0ms / network 0ms)

This is affect third party tooling which does exact comparisons for validation.

Jira issue: CRDB-27348

Epic CRDB-27601

[otan]

Root cause is here:

https://github.com/cockroachdb/apd/blob/3bce48c0644486e12c96bbeaee4d77e097777527/format.go#L69-L73

Since we have a 0 coefficient this gets triggered.

---

Consider the go program, which replicates the logic:

package main

import (
	"fmt"

	"github.com/cockroachdb/apd/v3"
)

func main() {
	for _, str := range []string{"0.000000", "0.0000000", "1.000000000"} {
		d, _, err := apd.NewFromString(str)
		if err != nil {
			panic(err)
		}
		fmt.Printf("##### %s\n", str)
		digits := d.Coeff.Append([]byte{}, 10)
		fmt.Printf("decimal info: form: %v, neg: %v, exp: %d, co: %v\n", d.Form, d.Negative, d.Exponent, d.Coeff)
		fmt.Printf("digits = %s: %d\n", digits, len(digits))
		adj := int(d.Exponent) + (len(digits) - 1)
		const adjExponentLimit = -6
		fmt.Printf("adj: %d, d.Exponent <= 0 && adj >= adjExponentLimit: %t\n\n", adj, d.Exponent <= 0 && adj >= adjExponentLimit)
	}
}

output:

##### 0.000000
decimal info: form: Finite, neg: false, exp: -6, co: {<nil> [0 0]}
digits = 0: 1
adj: -6, d.Exponent <= 0 && adj >= adjExponentLimit: true

##### 0.0000000
decimal info: form: Finite, neg: false, exp: -7, co: {<nil> [0 0]}
digits = 0: 1
adj: -7, d.Exponent <= 0 && adj >= adjExponentLimit: false

##### 1.000000000
decimal info: form: Finite, neg: false, exp: -9, co: {<nil> [1000000000 0]}
digits = 1000000000: 10
adj: 0, d.Exponent <= 0 && adj >= adjExponentLimit: true

I think we need to special case 0 somehow in this logic.

[otan]

Also not sure apd is the right place to fix it. Fixing it there (by making apd.Decimal.Coeff correct when returning digits) fails the gda_test tests:

diff --git a/decimal_test.go b/decimal_test.go
index 931b04a..2120577 100644
--- a/decimal_test.go
+++ b/decimal_test.go
@@ -447,6 +447,11 @@ func TestFormat(t *testing.T) {
 			f: "000",
 			g: "0e+2",
 		},
+		"0E-9": {
+			e: "0e-9",
+			f: "0.000000000",
+			g: "0.000000000",
+		},
 	}
 	verbs := []string{"%e", "%E", "%f", "%g", "%G"}
 
diff --git a/format.go b/format.go
index 83dfed9..84e805b 100644
--- a/format.go
+++ b/format.go
@@ -59,6 +59,12 @@ func (d *Decimal) Append(buf []byte, fmt byte) []byte {
 
 	var scratch [16]byte
 	digits := d.Coeff.Append(scratch[:0], 10)
+	// Correctly format the 0 digits.
+	if len(digits) == 1 && digits[0] == '0' && d.Exponent < 0 {
+		for i := int32(0); i < -d.Exponent; i++ {
+			digits = append(digits, '0')
+		}
+	}
 	switch fmt {
 	case 'e', 'E':
 		return fmtE(buf, fmt, d, digits)
diff --git a/sql_test.go b/sql_test.go
index 8660765..af0155c 100644
--- a/sql_test.go
+++ b/sql_test.go
@@ -75,4 +75,16 @@ func TestSQL(t *testing.T) {
 	if nd.Valid {
 		t.Fatal("expected null")
 	}
+	
+	var g Decimal
+	if err := db.QueryRow("select 0::decimal(19,9)").Scan(&g); err != nil {
+		t.Fatal(err)
+	}
+	zeroD, _, err := NewFromString("0.000000000")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if g.String() != zeroD.String() {
+		t.Fatalf("expected 0::decimal(19.9) pg value %s match, found %s", g.String(), zeroD.String())
+	}
 }

@rafiss penny for your thoughts?

[otan]

After #102220, not sure how possible this is to fix. It goes really deep as the "Decimal.String()" method is called EVERYWHERE (e.g. execution code). Fixing it exhaustively means fixing it at the apd layer.

[awoods187]

@dikshant PTAL and see if you can get this prioritized

[otan]

I can confirm with #102299 which has cockroachdb/apd#128 in, AWS DMS validation works as expected for this case.

It's a somewhat weird fix as cockroachdb/apd doesn't do the thing I think it should do (following the RDA spec, 0.000000000 should be 0 not 0E-9), but making it do something even less than the thing it should do seems worse.

[rafiss]

closed by #102299