Wouldn't String be a better serialization format for BigInteger and BigDecimal?
Closed this issue · 4 comments
Hi!
I saw that BigInteger is serialized as long and BigDecimal as double. But they can easily overflow long and double (that's why they exist ;)). Wouldn't it be better to serialize both as String?
Greetings,
Christian
Yes that's right. I am not 100% satisfied with the current solution. At the time of the initial implementation I was guided by Jackson, which also serialize BigInteger and BigDecimal as numbers.
On the one hand, this is not really a big deal because you can just register a custom VPackSerializer when you need to serialize them as String, on the other hand we might want to offer a more convenient way to change the serialized type for BigInteger and BigDecimal.
A quick search on google shows what possibilities you have with Jackson.
- converting the type in the getter (no option for us because we're not using setter/getter for serialization)
- use a custom serializer by registering a module (which we can already do)
- use a custom serializer by using annotation
JsonSerialize(providing that kind of annotation is already on my todo list)
I am open to suggestions. Should we change the default to String or just document how the default behavior is and how to easily change it with a custom VPackSerializer.
What we can do in any case is to make the deserializer for BigInteger and BigDecimal compatible with both number and String.
best
Mark
That's not an easy question. But considering that the reason why BigInteger and BigDecimal exist is that they can store much larger and more accurate numbers, then we have to ask if it makes sense to reduce them to long and double. Doesn't this make the use of both senseless?
What we can do in any case is to make the deserializer for BigInteger and BigDecimal compatible with both number and String.
I totally agree with that.
My first point against this change was that you can't work properly in AQL with this anymore, but then I realized that you can type cast it in AQL with TO_NUMBER(value) but then you will have data loss. But that same argument is true when we save them as long and double :-)
So I think, from the point of avoiding data loss, we should serialize it as String 👍
With the change on the deserializers I also see no real pain point with backward compatibility.
fixen in 1.1.0