serde-arbitrary-precision without serde-float

[drdo]

It's a little surprising to me that enabling serde-arbitrary-precision by itself does nothing
and requires serde-float to also be enabled.

Would you be open to changing this so that enabling serde-arbitrary-precision had the same behaviour
as enabling serde-arbitrary-precision and serde-float currently? I'm happy to submit a PR.

Or I'd enjoy hearing the rationale behind this :)

On a related topic, it seems that you are trying to promote the use of the with-* features instead.
The trouble is, unless I'm missing something, these don't seem very ergonomic to use if you have a Decimal inside other data structures and require you to essentially implement a whole custom serializer/deserializer.
What would be the typical way to use these if you have for example a struct field with a HashMap<i64, Vec<Decimal>>?

[paupino]

Hmm, I see what you're talking about. To describe this a bit - if we had the following dependencies:

[dependencies]
serde = { version = "*", features = ["derive"] }
serde_json = "*"
rust_decimal = { version = "*", default-features = false, features = ["serde-arbitrary-precision"] }
rust_decimal_macros = "*"

And this code:

use rust_decimal_macros::dec;

#[derive(serde::Serialize, serde::Deserialize, Debug)]
struct RatioWithout {
    value: rust_decimal::Decimal,
}


#[derive(serde::Serialize, serde::Deserialize, Debug)]
struct RatioWith {
    #[serde(with = "rust_decimal::serde::arbitrary_precision")]
    value: rust_decimal::Decimal,
}

fn main() {
    let ratio_with = RatioWith {
        value: dec!(3.14159),
    };
    let ratio_without = RatioWithout {
        value: dec!(3.14159),
    };
    println!("with: {}", serde_json::to_string(&ratio_with).unwrap());
    println!("without: {}", serde_json::to_string(&ratio_without).unwrap());

    let json_with: RatioWith = serde_json::from_str(r#"{"value":3.14159}"#).unwrap();
    let json_without: RatioWithout = serde_json::from_str(r#"{"value":3.14159}"#).unwrap();
    println!("json_with: {:?}", json_with);
    println!("json_without: {:?}", json_without);
}

The output is:

with: {"value":3.14159}
without: {"value":"3.14159"}
json_with: RatioWith { value: 3.14159 }
json_without: RatioWithout { value: 3.14159 }

The question is about serialization of the value given that we've enabled the "default" feature, and not just the "with" variant.

I think this goes back to the intention of the with vs non-with pattern. The general idea for the with-* pattern is to give you greater granularity of how an API is consumed/generated. This is very useful for consuming third-party APIs since they may serialize/expect numbers differently that we'd like to represent them.

The non-"with" pattern is intended to be a blanket controlling feature for the decimal library. i.e. you are the library publishing/creating numbers and this is how you want to do it. Alternatively, it can provide a "default" serialization/deserialization format (you can always override with with-* variants).

Consequently, I do think this is unexpected behavior - the non-with pattern should behave the same as if with was provided. It should provide a blanket/default rule for ALL decimal types being used.

So a PR would definitely be useful and much appreciated - thank you @drdo !

Regarding the promotion of with-* features - perhaps I need to revise the documentation a bit more. As I've tried to describe above: they are there for greater granularity of serialization/deserialization support but the non with patterns are there for the "default" rules. There are a time and a place for each. I'm guessing that the note on the readme is coming across a bit too strongly.

The serde features in this library are a bit of a complex matrix - I think in general they probably deserve their own readme section to better describe how to use them (or how they can be used).

[drdo]

Do you think it ever makes sense to enable several of serde-str, serde-float and serde-arbitrary-precision at the same time? Should these be mutually exclusive?

Maybe even delete serde-str (or turn into a noop) and make that the default?

[paupino]

Do you think it ever makes sense to enable several of serde-str, serde-float and serde-arbitrary-precision at the same time? Should these be mutually exclusive?

I think they should probably be mutually exclusive. I'd have to have a bit of a deeper dive regarding serde-str though - there are some explicit scenarios (i.e. bincode) where I believe the type needs to be explicit. I think the default is a bit more relaxed for deserialization but I could be wrong.

Either way - simplifying these feature combinations would definitely help reduce the complexity of the area!

[bjorn]

Just adding my two cents. I came upon this issue after looking into a strange deserialization issue in my application. When inserting the above code by @paupino as my main, leaving the rest of my app and its dependencies intact, I would run into the following error:

❯ cargo run
   Compiling serde_json v1.0.108
   Compiling decimal v0.1.0 (/home/bjorn/playground/decimal)
    Finished dev [unoptimized + debuginfo] target(s) in 1.10s
     Running `target/debug/decimal`
with: {"value":3.14159}
without: {"value":"3.14159"}
thread 'main' panicked at src/main.rs:25:77:
called `Result::unwrap()` on an `Err` value: Error("invalid type: map, expected a Decimal type representing a fixed-point number", line: 1, column: 16)
stack backtrace:
   0: rust_begin_unwind
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/std/src/panicking.rs:595:5
   1: core::panicking::panic_fmt
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/panicking.rs:67:14
   2: core::result::unwrap_failed
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/result.rs:1652:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/result.rs:1077:23
   4: decimal::main
             at ./src/main.rs:25:32
   5: core::ops::function::FnOnce::call_once
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

I was stuck on this for quite a while since I was sure my code had worked before and could not find any way to fix it. In the end, using cargo tree -e normal,build -f "{p} [{f}]" I noticed some other crate in my dependencies must be enabling the arbitrary_precision feature of serde_json. Indeed the error happens with that combination:

rust_decimal = { version = "1.33.1", features = ["serde-with-float"] }
rust_decimal_macros = "1.33.1"
serde = { version = "1.0.193", features = ["derive"] }
serde_json = { version = "1.0.108", features = ["arbitrary_precision"] }

And the fix appears to be to enable the serde-arbitrary-precision feature for rust_decimal.

What am I trying to say is that while I needed to enable serde-arbitrary-precision in my project to fix this runtime deserialization error, I actually do not want to enable serde-float (since I'm serializing as string in my app) nor do I have a need for serde-with-float currently.

[paupino]

I think I originally misunderstood the context of this question! Originally, I thought serde_json/arbitrary_precision was an identity like feature - that is it could parse a float with arbitrary precision during deserialization so required a float to serialize with for identity-like operations. HOWEVER, while trying to improve the documentation I discovered that arbitrary precision supports either a float-like or string-like number.

I tend to agree about serializing as a string by default and instead only serializing as a float on demand. I'll look into making this change however I need to do a bit of planning about how to handle this change since it'd technically be breaking.

In the meantime, as a workaround - you can do this:

Cargo.toml:

rust_decimal = { version = "*", features = ["serde-with-arbitrary-precision", "serde-with-str"] }

    #[derive(serde::Serialize, serde::Deserialize)]
    struct Total {
        #[serde(
            deserialize_with = "rust_decimal::serde::arbitrary_precision::deserialize",
            serialize_with = "rust_decimal::serde::str::serialize"
        )]
        value: Decimal,
    }

    let total = Total { value: dec!(1.23) };
    let serialized = serde_json::to_string(&total)?;
    assert_eq!(r#"{"value":"1.23"}"#, serialized);

    let deserialized: Total = serde_json::from_str(&serialized)?;
    assert_eq!(dec!(1.23), deserialized.value);

    let deserialized: Total = serde_json::from_str(r#"{"value":1.23}"#)?;
    assert_eq!(dec!(1.23), deserialized.value);

This forces arbitrary precision to be used for deserialization while keeping string as a serialization format. If you wanted to slightly make it less "wordy" you could also do the following to keep the "default" serialization as string but only deserialize with arbitrary precision:

Cargo.toml:

rust_decimal = { version = "*", features = ["serde-with-arbitrary-precision"] }

    #[derive(serde::Serialize, serde::Deserialize)]
    struct Total {
        #[serde(
            deserialize_with = "rust_decimal::serde::arbitrary_precision::deserialize",
        )]
        value: Decimal,
    }

    let total = Total { value: dec!(1.23) };
    let serialized = serde_json::to_string(&total)?;
    assert_eq!(r#"{"value":"1.23"}"#, serialized);

    let deserialized: Total = serde_json::from_str(&serialized)?;
    assert_eq!(dec!(1.23), deserialized.value);

    let deserialized: Total = serde_json::from_str(r#"{"value":1.23}"#)?;
    assert_eq!(dec!(1.23), deserialized.value);

[bjorn]

@paupino The concern I was addressing with my comment was in response to the initial question raised by this issue:

Would you be open to changing this so that enabling serde-arbitrary-precision had the same behaviour
as enabling serde-arbitrary-precision and serde-float currently?

Since enabling serde-float would cause all my decimals to get serialized as floats by default, but I needed to enable serde-arbitrary-precision for other reasons (to fix the above compile error), this suggested change would be undesirable for my project.

[paupino]

@paupino The concern I was addressing with my comment was in response to the initial question raised by this issue:

Would you be open to changing this so that enabling serde-arbitrary-precision had the same behaviour
as enabling serde-arbitrary-precision and serde-float currently?

Since enabling serde-float would cause all my decimals to get serialized as floats by default, but I needed to enable serde-arbitrary-precision for other reasons (to fix the above compile error), this suggested change would be undesirable for my project.

Cool - I actually tend to agree with you here. For this behavior I'd expect a little bit of the reverse logic - enable float serialization on demand. e.g.

Cargo.toml:

rust_decimal = { version = "*", features = ["serde-arbitrary-precision", "serde-with-float"] }

    #[derive(serde::Serialize, serde::Deserialize)]
    struct Total {
        #[serde(
            serialize_with = "rust_decimal::serde::float::serialize",
        )]
        value: Decimal,
    }

    let total = Total { value: dec!(1.23) };
    let serialized = serde_json::to_string(&total)?;
    assert_eq!(r#"{"value":1.23}"#, serialized);

    let deserialized: Total = serde_json::from_str(&serialized)?;
    assert_eq!(dec!(1.23), deserialized.value);

    let deserialized: Total = serde_json::from_str(r#"{"value":"1.23"}"#)?;
    assert_eq!(dec!(1.23), deserialized.value);

I'd rather keep strong serialization by default with the ability to relax it only if need be.

[tomasvdw]

My suggestion would be to deprecate the non-with features. They are incorrect as they do not comply to requirement of features being additiive.

See: https://doc.rust-lang.org/cargo/reference/features.html#feature-unification

This can result in hard-to-find regression bugs when dependent libraries use conflicting features.