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

Support i128 and u128 #846

Closed
tustvold opened this issue Jan 14, 2022 · 7 comments
Closed

Support i128 and u128 #846

tustvold opened this issue Jan 14, 2022 · 7 comments

Comments

@tustvold
Copy link

Currently by default this crate supports numbers up to 64-bits. For larger types it is necessary to enable the arbitrary_precision feature, which treats numbers as strings. Whilst this works, it can have unintended consequences as it changes the semantics of how integers are fed into the serde machinery - see #845.

I wonder if serde_json might add support for 128-bit types within Number and the associated serializer implementations on platforms that support such types - e.g. by using serde_if_integer128. This would allow crates to use 128-bit types without needing to opt-in to arbitrary_precision.

I would be happy to contribute a PR if this is an acceptable course of action?

@alextes
Copy link

alextes commented Jul 13, 2022

To expand a little, I'm using i128 numbers, I want to put them into the DB as JSON, I'm using sqlx and sqlx using query! would like me to provide a serde_json Value but as written above trying to serialize an i128 leads to an error.

I'll first try to find a workaround and share it, but it would be lovely to know why serializing i128 isn't supported, what possible routes forward are available, and which would be welcome as a PR.

@alextes
Copy link

alextes commented Jul 13, 2022

After a bit of a wild goose chase. serde_json is happy to serialize to String, even if not to Value. Working with passing a string to the DB that is hinted as being JSON is easy with sqlx. So for others, a decent workaround might be to serialize to string and have whatever is on the other side deserialize as usual.

This also means that my issue, in the end, was converting i128 to Value which I feel is pretty minor.

@dtolnay
Copy link
Member

dtolnay commented Jul 13, 2022

I think Number and Value are fine as currently implemented. If someone needs a Value with bigger numbers, they can either enable arbitrary_precision to make Value support bigger numbers, or they can use a different Value type. The non-arbitrary_precision Value is just an ordinary Rust enum with a suitable Serialize and Deserialize impl so there is nothing special about it being in serde_json. You can define your own in a different crate with whatever representation/implementation you prefer.

@dtolnay dtolnay closed this as completed Jul 13, 2022
@alextes
Copy link

alextes commented Jul 13, 2022

arbitrary_precision doesn't seem documented anywhere. If you feel that's the best way to handle this scenario would you be interested in a PR for that?

RE: or don't use Value if you want bigger numbers. Fair enough, no reason for sqlx to rely on serde_jsons Value` only, and I believe one can already define their own "sql-izable" type that one can then pass to the query builder.

I would still be interested to hear @dtolnay , perhaps also helpful to link people to in the future, why not support the larger numbers? It seems JSON has no opinion on how large numbers can be, why does serde_json?

Thanks for the quick reply!

@dtolnay
Copy link
Member

dtolnay commented Jul 13, 2022

JSON doesn't not have an opinion, JSON has the opinion that implementations define the limit on their range and precision of numbers.

This specification allows implementations to set limits on the range and precision of numbers accepted.

If anything, it appears to consider i54 the limit of what JSON-producing software generally gets to expect will work.

Since software that implements IEEE 754 binary64 (double precision) numbers is generally available and widely used, good interoperability can be achieved by implementations that expect no more precision or range than these provide.

Note that when such software is used, numbers that are integers and are in the range [-(2**53)+1, (2**53)-1] are interoperable in the sense that implementations will agree exactly on their numeric values.

https://www.rfc-editor.org/rfc/rfc8259

You are commenting as though JSON has the opinion that implementations ought to support arbitrarily big numbers, which is not reflected in the text.

On the Rust side of things, I think those people expecting to put u128 inside Value arrive at that expectation because of Rust supporting u128-sized integer literals in the syntax, but that really doesn't have bearing on JSON as a data format. Analogously if Rust were to eventually add support for u256, u512, u1024, and u2048 sized integer literals for a bizarre niche use case, we in serde_json would not immediately feel the urge to bloat all users of serde_json by blowing up the size of our Number type.

@alextes
Copy link

alextes commented Jul 14, 2022

Reading the section on numbers I'd agree with you! Accepting <= i54 and otherwise f64 seems most reasonable actually going by the spec but then making that i64 and f64, as I believe is the case now, might be more practical.

You are commenting as though JSON has the opinion that implementations ought to support arbitrarily big numbers, which is not reflected in the text.

Ah, I didn't mean to imply as much. Only that one may when the implementation supports it, reading the full section I'd say ought or should is be double precision float compatible. Perhaps in a far-future scenario serde_json could default to f64 but allow the user to use pick any number the rust implementation supports. I'm not suggesting to implement such a thing now.

Thanks for the rationalization, for others reading along I'd then update my conclusion to be: stay f64 compatible, that is f64 or integers that fit within i54 so f64 implementations deserializing will read the exact value you wrote. Everything else should be a string if you care about precision.

@appetrosyan
Copy link

My two cents. The reason why people expect u128 to be supported is because Rust's way of communicating is with traits, and #[derive(Serialize, Deserialize)] doesn't, in fact, signal that these values can fail to deserialize in any way in some formats. It could raise a warning. It could standardise the Deserialization but not the Serialization.

You are assuming that people know that values in JSON are implicitly (as per the fact that it is Java Script Object Notation), and immediately see that the built-in type u128 that implements Deserialize is somehow going to fail for an integral value 1, which is well within JSON spec. Given that it's a run-time failure, it's even more odd, because then you could differentiate between values that are outside the spec and within spec and only fail for the values which would cause a problem in JavaScript (assuming you ignore BigInt).

So IMO, there were three possible architectures that could have been adopted which would have avoided this issue.

  1. serde support for u128 is opt-in. The user knows that they need to be careful around u128 at the point of adding #[derive(Deserialize)].
  2. serde_json support for u128 is opt-out. This could lead to problems with clients that support JSON but not wide integers. Admittedly this is not the best solution, but in all fairness, you kinda have to look for implementations that don't support u128. Difference being that a serde_json based client is no longer one of those implementations that err on the side of "caution".
  3. serde_json supports u128 but errors out if the value falls outside u54. In reality the support could be much more fine-grained, because not all values that don't fit into u54 are also unrepresentable in JSON. They can sometimes lead to loss of precision and failure if and only if there was loss of precision is perfectly acceptable in most circumstances. In that situation you're allowed to cast a u54 to u128 by deserializing, and downcast u128 to f64 if and only if that cast is lossless. Admittedly it's a bit more work, but this seems to point the user exactly at the problem if and when it occurs.
  4. serde and serde_json do not support u128 at all. While this might seem radical, it is not unheard of by any stretch of the imagination, and actually achieves the goals outlined here for not supporting u128. Case in point, repr(C) doesn't support u128 either. Given semver guarantees, the ship has sailed on this one.

All of the above solutions make the user aware of the problem, in the situation where something like
serde-rs/serde#2376 is not supported, and the user is left guessing why their untagged enum fails to deserialize for a value of u128 but not u64. If u64 were rejected universally, it would have been a different situation, because then it would have at least hinted at the fact that it has to do with JavaScript's domain-specific decision to use f64 for all numerical values except BigInt.

@serde-rs serde-rs locked and limited conversation to collaborators Mar 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

4 participants