From ffd11b1d1ea290d5ebedb620e2a59e8b62897fb2 Mon Sep 17 00:00:00 2001 From: Oliver Tan Date: Mon, 15 May 2023 10:27:22 +1000 Subject: [PATCH] format: format 0E-x where x > 6 like PostgreSQL does Officially in the GDA spec, formatting a 0 with any decimal places should be 0. However, this code doesn't do that - it formats the number of 0s up to 6, and then `0E-exponent` afterwards. To make this more "PG" compatible, we fix this by ensuring we display as many 0s as the exponent allows in the `.String()` case. --- .github/workflows/go.yml | 6 +++--- decimal_test.go | 6 ++++++ format.go | 12 +++++++++++- gda_test.go | 22 ++++++++++++++++++++-- sql_test.go | 12 ++++++++++++ 5 files changed, 52 insertions(+), 6 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 8b9bf79..ad755d8 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -61,10 +61,10 @@ jobs: run: go vet -unsafeptr=false ./... - name: 'Staticcheck' - # staticcheck requires go1.17. - if: ${{ matrix.arch == 'x64' && matrix.go >= '1.17' }} + # staticcheck requires go1.19. + if: ${{ matrix.arch == 'x64' && matrix.go >= '1.19' }} run: | - go install honnef.co/go/tools/cmd/staticcheck@latest + go install honnef.co/go/tools/cmd/staticcheck@v0.4.3 staticcheck ./... - name: 'GCAssert' diff --git a/decimal_test.go b/decimal_test.go index 931b04a..281b84f 100644 --- a/decimal_test.go +++ b/decimal_test.go @@ -447,6 +447,12 @@ func TestFormat(t *testing.T) { f: "000", g: "0e+2", }, + "0E-9": { + e: "0e-9", + f: "0.000000000", + g: "0.000000000", + G: "0.000000000", + }, } verbs := []string{"%e", "%E", "%f", "%g", "%G"} diff --git a/format.go b/format.go index 83dfed9..c40fc47 100644 --- a/format.go +++ b/format.go @@ -65,9 +65,19 @@ func (d *Decimal) Append(buf []byte, fmt byte) []byte { 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 { + digitLen += int(-d.Exponent) + } // See: http://speleotrove.com/decimal/daconvs.html#reftostr const adjExponentLimit = -6 - adj := int(d.Exponent) + (len(digits) - 1) + adj := int(d.Exponent) + (digitLen - 1) if d.Exponent <= 0 && adj >= adjExponentLimit { return fmtF(buf, d, digits) } diff --git a/gda_test.go b/gda_test.go index a05f622..392f437 100644 --- a/gda_test.go +++ b/gda_test.go @@ -683,8 +683,26 @@ func gdaTest(t *testing.T, path string, tcs []TestCase) { if strings.HasPrefix(tc.Result, "SNAN") { tc.Result = "sNaN" } - if !strings.EqualFold(s, tc.Result) { - t.Fatalf("expected %s, got %s", tc.Result, s) + expected := tc.Result + // Adjust 0E- or -0E- tests to match PostgreSQL behavior. + // See: https://github.com/cockroachdb/cockroach/issues/102217. + if pos, neg := strings.HasPrefix(expected, "0E-"), strings.HasPrefix(expected, "-0E-"); pos || neg { + startIdx := 3 + if neg { + startIdx = 4 + } + p, err := strconv.ParseInt(expected[startIdx:], 10, 64) + if err != nil { + t.Fatalf("unexpected error converting int: %v", err) + } + expected = "" + if neg { + expected = "-" + } + expected += "0." + strings.Repeat("0", int(p)) + } + if !strings.EqualFold(s, expected) { + t.Fatalf("expected %s, got %s", expected, s) } return } diff --git a/sql_test.go b/sql_test.go index 8660765..b000326 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()) + } }