bson-rust icon indicating copy to clipboard operation
bson-rust copied to clipboard

RUST-36 Support of Decimal128 type

Open zonyitoo opened this issue 8 years ago • 16 comments

According to BSON 1.1 Spec, it introduced a new type decimal128.

But currently Rust doesn't support d128 type natively.

Related links

  1. d128 crate: https://github.com/alkis/decimal
  2. Official support RFC: https://github.com/rust-num/num/issues/8

zonyitoo avatar Feb 04 '17 04:02 zonyitoo

Thoughts? @kyeah

zonyitoo avatar Feb 04 '17 13:02 zonyitoo

nice, just one comment; looks great otherwise.

kyeah avatar Feb 04 '17 18:02 kyeah

I believe crates.io doesn't let you publish a crate that has both git and crates.io dependencies. Is there any chance that the author of the decimal crate might accept a PR?

EDIT: Oops, that was the "one comment"; never mind!

saghm avatar Feb 04 '17 18:02 saghm

I have already submitted a PR to https://github.com/alkis/decimal/pull/23 . I will remove that git dep after this PR is accepted.

zonyitoo avatar Feb 05 '17 01:02 zonyitoo

I see there is a commit with decimal128 support already. Are you still thinking of merging that commit in? Would love to see that happen!

lrlna avatar Jan 05 '19 15:01 lrlna

Don't know if it is the best way to support decimal 128 in Rust, yet.

Rust is now supported i128 and u128. Maybe in the future it will support d128?

zonyitoo avatar Jan 06 '19 02:01 zonyitoo

@zonyitoo by the best way do you mean you're not sure if using the decimal crate is a good idea, or is this more about whether the support is up to the spec?

it's possible core will support d128, but I feel like that might be a long way off :(. In the meantime would be cool to have a workaround \o/

lrlna avatar Jan 07 '19 12:01 lrlna

Yes, I agree. I would take sometime to make this commit works with the master.

zonyitoo avatar Jan 07 '19 15:01 zonyitoo

@zonyitoo if you feel like it's more work than you have capacity for, I can also help out! I just didn't want to step on your toes, since there was already a commit going.

lrlna avatar Jan 09 '19 12:01 lrlna

@lrlna That would be great! I don't have spear time in the recent future :(

zonyitoo avatar Jan 10 '19 02:01 zonyitoo

We can use https://jira.mongodb.org/browse/RUST-36 to track the work done for this issue

saghm avatar Feb 10 '20 16:02 saghm

I filed https://github.com/mongodb-rust/mongodb-schema-parser/pull/35 before I saw this conversation. I am trying to parse a collection's schema using the schema parser. However because mongodb doesn't build with the feature = ["decimal128"] from the bson crate, I'm encountering a non-exhaustive pattern as per below.

    Checking mongodb v0.9.1
error[E0004]: non-exhaustive patterns: `&Decimal128(_)` not covered
   --> C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\mongodb-0.9.1\src\bson_util.rs:128:11
    |
128 |     match val {
    |           ^^^ pattern `&Decimal128(_)` not covered
    |
    = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

This is with the below Cargo.toml

[dependencies]
bson = { version = "0.14", features = ["decimal128"] }
mongodb = "0.9.1"
# this contains the patch to use bson 0.14 on the schema parser
mongodb-schema-parser = { git = "https://github.com/nevi-me/mongodb-schema-parser", branch = "update-bson" }

I don't need decimal128 support, so I'll fork the mongodb crate and add the extra pattern for now.

nevi-me avatar Feb 23 '20 05:02 nevi-me

I tried to read data from mongodb collection that uses decimal128 but it panics

thread 'main' panicked at 'Attempted conversion of invalid data type: Decimal128(...)', C:\Users\Konrad.cargo\registry\src\github.com-1ecc6299db9ec823\bson-2.0.0-beta.1\src\bson.rs:587:18 stack backtrace:...

Is there any workaround or it's simply not possible with rust's mongodb library?

EDIT:

I had to explicitly add this to [dependencies]:

bson = { version = "2.0.0-beta.1", features = ["decimal128"] }

bugproof avatar Jun 18 '21 11:06 bugproof

@bugproof This was a bug actually that has since been fixed in 2.0.0-beta.3 and 1.2.3. Note that the decimal128 feature flag has been removed in 2.0.0-beta.3 (see #282), so you'll need to remove that workaround. Deserialization from documents that contain decimal128 values should work just fine now though.

patrickfreed avatar Aug 06 '21 19:08 patrickfreed

Any update?

emreyalvac avatar Jun 07 '22 19:06 emreyalvac

Just checking in to see if there has been any update on this?

dotjon0 avatar Sep 09 '22 16:09 dotjon0

Could the MongoDB team provide feedback of where support for Decimal128 is at? This issue was originally raised in 2017 and with no recent updates.

A could of key points:

  1. MongoDB recommends Decimal128 as the best option for 'monetary' high precision values https://www.mongodb.com/docs/manual/tutorial/model-monetary-data/

  2. Rust is becoming a very mainstream language. Lack of Decimal128 support could block development teams from using MongoDB Atlas if they have adopted rust.

dotjon0 avatar Nov 02 '22 20:11 dotjon0

Hi @dotjon0, our current plan is to start implementation of this ~1 month from now.

kmahar avatar Nov 02 '22 20:11 kmahar

@kmahar thank you so much for the update, its very appreciated. Great to hear Decimal128 implementation is commencing in only a few weeks! Absolute fantastic news! Will look forward and thank you for making the time to update us.

dotjon0 avatar Nov 02 '22 20:11 dotjon0

@kmahar hello 👋 ! I think there might be some interest in rust's standard library to have rust support decimal 128 implementation natively. If you end up writing this implementation from scratch, would be super cool if you could contribute it back to rust itself :) There is already a u128/i128, so std is close!

lrlna avatar Nov 08 '22 12:11 lrlna

hey @lrlna! 🙂 I think to start this type would likely only support converting back and forth between string <-> decimal128 byte representations, and in the future we may try to support conversion to/from other Rust numeric types. If there was a std decimal128 type we’d want to support interop with that, and we could potentially switch to using it in lieu of our type if it supported BID coefficient encoding. We’d be very happy to contribute our string <-> bytes logic upstream if that helps along an implementation, but at this time I think it’s unlikely we’ll end up writing a full-featured type ourselves that directly supports math like I assume a std type would. However, if that changes we’ll definitely keep this in mind!

kmahar avatar Nov 09 '22 16:11 kmahar

I think to start this type would likely only support converting back and forth between string <-> decimal128 byte representations,

amazing, exactly what we need! Thanks so much @kmahar

dotjon0 avatar Nov 18 '22 17:11 dotjon0

Sorry for the long delay here, just wanted to let people know that implementation of conversion to and from human-readable strings for the Decimal128 type has started, and we expect this to go out in the next driver release. If there's other functionality that would be particularly useful, please let us know! It's unlikely to be part of the upcoming feature addition but will definitely inform future priorities.

abr-egn avatar Feb 02 '23 15:02 abr-egn

We just released version 2.6.0 of the bson crate, which includes support for conversion between Decimal128 values and human-readable strings 🙂

abr-egn avatar Mar 14 '23 17:03 abr-egn

We just released version 2.6.0 of the bson crate, which includes support for conversion between Decimal128 values and human-readable strings 🙂

amazing @abr-egn thank you so much!

dotjon0 avatar Apr 14 '23 16:04 dotjon0