ericlagergren / decimal

A high-performance, arbitrary-precision, floating-point decimal library.
https://godoc.org/github.com/ericlagergren/decimal
BSD 3-Clause "New" or "Revised" License
516 stars 61 forks source link

Wrapping for database use #143

Open aarondl opened 4 years ago

aarondl commented 4 years ago

Hi @ericlagergren, more trouble from the sqlboiler world :)

Issue https://github.com/volatiletech/sqlboiler/issues/607 details the failure to be able to insert our wrapper type into the database anymore. After reconstructing a small sample that looks like this:

    var inDec types.Decimal
    inDec.Big, _ = new(decimal.Big).SetString("1.0")

    _, err = db.Exec(`insert into objects (value) values ($1);`, &inDec)
    if err != nil {
        panic(err)
    }

    var outDec types.Decimal
    row := db.QueryRow(`select value from objects limit 1;`)
    if err != nil {
        panic(err)
    }
    err = row.Scan(&outDec)
    if err != nil {
        panic(err)
    }

    fmt.Println(outDec.String())

I've found the following results:

I don't pretend the blame is not on our end. As we've been marshalling with this basic string-based wrapper for some time now: https://github.com/volatiletech/sqlboiler/blob/master/types/decimal.go#L97-L152

Admittedly quite the hack, but it seems to have worked until now. I suspect the addition of the decomposer interface implementation in the master branch is key (since this issue only hit sqlboiler users on Go 1.13).

What's confusing is that you'd expect lib/pq to pick up the decomposer interface implementation of the underlying decimal and use it correctly. I was hoping to see if you had any insights as to why this might not be working.

ericlagergren commented 4 years ago

So, this is what's up: https://github.com/golang/go/issues/34315

The lib/pq package currently does not recognize the decomposer interface. I've opened an issue there: https://github.com/lib/pq/issues/904

At first blush, I'm not sure whether this is a bug in lib/pq or the stdlib. If lib/pq adds support then the issue will be resolved, but the stdlib could also fix the issue by not having the conversion code cut out early if it sees a decimalDecomposer that's also a driver.Valuer.

I'm not sure there's much I could do on my end other than removing decimalDecomposer support, but it seems short sighted to do that because other packages like lib/pq are out of date. And I'm sure if I removed that interface I'd get a bug report asking me to add it back in. :)

Ideas?

ericlagergren commented 4 years ago

I'm gonna try opening a PR in lib/pq later this week. https://github.com/ericlagergren/pq/commit/92294e9b1445a4d38ecf46bcf03b780ca34b6d07

aarondl commented 4 years ago

Sounds good. As for now I've informed sqlboiler users to stick to tagged versions, which they should have been doing anyway. But go get makes it too convenient to not do that and many people are still not using go modules (for very good reasons mind you).