Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util/decimal: Investigate EricLagergren/decimal package #6009

Closed
nvanbenschoten opened this issue Apr 12, 2016 · 10 comments
Closed

util/decimal: Investigate EricLagergren/decimal package #6009

nvanbenschoten opened this issue Apr 12, 2016 · 10 comments
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) C-investigation Further steps needed to qualify. C-label will change. C-performance Perf of queries or internals. Solution not expected to change functional behavior.
Milestone

Comments

@nvanbenschoten
Copy link
Member

This library looks really good, and could be a justifiable alternative to our current inf/dec library.

Specially, reasons to consider switching libraries are that this new package:

  • stores small integers inside a compact int64, speeding up operations and potentially avoiding allocations
  • includes support for a larger number of arithmetic methods (Log, Exp, etc.)
    so we could remove our custom implementations
  • uses a Context to define how arithmetic operations react (common in a lot of other languages)
  • even comes with a FizzBuz function!

Either way, we should continue to monitor golang/go#12127.

@nvanbenschoten nvanbenschoten self-assigned this Apr 12, 2016
@nvanbenschoten nvanbenschoten added the C-investigation Further steps needed to qualify. C-label will change. label Apr 12, 2016
@nvanbenschoten nvanbenschoten added this to the Later milestone Apr 12, 2016
@ericlagergren
Copy link

Hey! Just noticed this. Feel free to ask about anything you want. :)

@tamird
Copy link
Contributor

tamird commented Aug 8, 2016

Also, our current decimal library appears not to handle scientific notation, while PG does. EricLagergren/decimal also handles scientific notation, which is good.

@tamird
Copy link
Contributor

tamird commented Aug 8, 2016

@ericlagergren it seems that you don't have an equivalent of https://godoc.org/gopkg.in/inf.v0#Dec.UnscaledBig, which we need for our serialization of decimals.

@ericlagergren
Copy link

@tamird That could easily be added. Unless the decimal spec (http://speleotrove.com/decimal/decarith.html) has a name for the method, I'd probably lean towards a Mantissa method or something like that. But, yeah, could be easily added.

@tamird
Copy link
Contributor

tamird commented Aug 8, 2016

I haven't read the spec, but Mantissa SGTM.

@knz knz added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) C-performance Perf of queries or internals. Solution not expected to change functional behavior. labels Aug 20, 2016
@tamird
Copy link
Contributor

tamird commented Aug 31, 2016

Note: there are some optimizations in 72a5c19 which are baked into github.com/EricLagergren/decimal and should be removed when this is implemented.

@nvanbenschoten
Copy link
Member Author

apd is CockroachDB's new Arbitrary-precision decimal library. The library fully replaced inf.Dec in #13551, and will be the decimal package we use going forward.

@tamalsaha
Copy link

@nvanbenschoten, do you mind elaborating why write a new library? I am trying to find a big.Decimal lib for some financial calculation. So, I am curious.

@ericlagergren
Copy link

ericlagergren commented Apr 12, 2017

@tamalsaha from a reddit thread: https://np.reddit.com/r/golang/comments/5zkzrq/apd_an_arbitraryprecision_decimal_package/dezaw3v/

@nvanbenschoten
Copy link
Member Author

@tamalsaha the reddit thread and blog post Eric linked to does a pretty good job explaining our rationale. The decision had less to do with specific shortcomings in other libraries and more to do with independent requirements for our use case, like panic-free operation and configurable precision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) C-investigation Further steps needed to qualify. C-label will change. C-performance Perf of queries or internals. Solution not expected to change functional behavior.
Projects
None yet
Development

No branches or pull requests

5 participants