search

paupino / rust-decimal

Decimal number implementation written in pure Rust suitable for financial and fixed-precision calculations.
https://docs.rs/rust_decimal/
MIT License
1.02k stars 183 forks source link

deserializes empty string into None Option #607

Closed gai6948 closed 11 months ago

gai6948 commented 1 year ago

Currently deserializing empty string will give an error regardless of using Decimal or Option<Decimal>.

This PR enables deserializing empty &str into None option when using crate::serde::str_option.

paupino commented 1 year ago

Thank you for your contribution!

While it is a simple change, I'm just thinking through whether this is breaking or not (i.e. need to be feature gated).

On one hand, it changes the meaning of "" - i.e. it would not have been able to be deserialized before. Does null really = ""? On the other hand, since it would not have worked before, is this an issue?

The safest approach would be to feature flag this, however we're already in a pretty confusing place with serde feature flags!

I'm curious to know whether anyone would consider this breaking in their application. I'll leave this PR open for a little bit for a "comment period" however assuming no input otherwise, I can look at merging this.

gai6948 commented 1 year ago

Thanks for the response @paupino ! Actually I was also thinking whether treating empty string as None is the right way to go, can't find such examples online though.

Wrongly parsing an empty string into Decimal is really bad, but in the case of Option<Decimal>, I would say users are already ready to handle null values anyways, so panicking is unnecessary