Support decimal precision for Document.FromJson and ToJson by using System.Text.Json instead of LitJson #3534
Conversation
…f LitJson. This supports additional precision for decimal values.
| writer.WriteRaw(stringValue); | ||
| break; | ||
| { | ||
| #if NETCOREAPP3_1 // WriteRawValue was added in .NET 6, but we need to write out Number values without quotes |
There was a problem hiding this comment.
Question: Does this mean for the .NET Framework target we'd try to use WriteRawValue too (which is only available in .NET 6)? Or is it fine because we have System.Text.Json as a dependency for net472?
There was a problem hiding this comment.
It's available in 4.7.2. At the time 3.1 was the only target we needed this workaround for.
| if (!json.IsObject) | ||
| var json = JsonDocument.Parse(jsonText, new JsonDocumentOptions | ||
| { | ||
| AllowTrailingCommas = true, |
There was a problem hiding this comment.
In your description you say
"A slight change in behavior is the JSON being past into the FromJson is more strictly parsed. Meaning it doesn't allow extra , for the last property in an object. LitJson would allow that. Given that isn't valid JSON this seems an acceptable change for V4."
but here you are allowing trailing commas. Did you set AllowTrailingCommas to true on purpose to keep the same behavior as litJson or did you mean to set this to false?
There was a problem hiding this comment.
Oh yeah. I copied the comment from Alex's original PR he must have added setting that property afterwards. I'll remove that comment from the PR description.
| break; | ||
| { | ||
| #if NETCOREAPP3_1 // WriteRawValue was added in .NET 6, but we need to write out Number values without quotes | ||
| using var document = JsonDocument.Parse(stringValue); |
There was a problem hiding this comment.
I think there is an alternative, simpler solution here which is to use something like:
var number = Convert.ToDouble(stringValue);
writer.WriteNumberValue(number);
This way, we don't need to have a preprocessor directive. But maybe I'm missing something and there is a reason why it was done this way.😅
There was a problem hiding this comment.
Is this what the user was complaining about for losing decimal precision? If that's the case then nvm!
There was a problem hiding this comment.
We would potentially lose precision trying to convert the string version of the number to the best fit .NET numeric type. Overall I don't think that will simplify the logic.
|
Wow, looking forward to this PR. Just stumbled upon JSON value 0.0000087 being converted to "8.7e-06" after applying Document.FromJson instead of staying "0.0000087" |
|
@Lanayx Glad this change will help you out. This is merged into our next major version which we are are currently on preview 4. We are hoping to a preview 5 within the next couple weeks that will include this change. |
Description
Taking over @ashovlin previous PR switching the
ToJsonandFromJsonmethods in DynamoDB to use System.Text.json instead of LitJson. #3447I have double check data type conversions like doubles, dates, bools and binary data comparing with V3 behavior and the behavior was consistent except for the expect use of
Decimalfor larger precision and the values being cast toDecimal.Motivation and Context
Avoid losing precision for floating point numbers with a lot of digits. It doesn't solve all use cases because .NET's
Decimaltype doesn't support as high an exponent asdoubledoes. If the floating point number is using a high exponent it will use a double and possibly lose precision. This is limitation of floating point numbers in .NET because .NET doesn't have a type like Java's BigDecimal.Testing
Had a successful dry run. Tests were added in @ashovlin initial work. Also did spot checking of data types.