large integer IDs are rounded
mstorus opened this issue · 7 comments
When viewing records whose field values are numbers that exceed JavaScript's Number.MAX_SAFE_INTEGER, the values are rounded to fit within a Number.
For example, these user_ids are incorrectly rounded to the nearest hundred:
You can see this rounding effect in a simple JavaScript evaluation:
$ node
> 1318796827993409899
1318796827993409800
I believe the correct solution is to make use of aws-sdk-js's wrapNumbers argument, described here:
https://github.com/aws/aws-sdk-js/blob/b232e4aa2827bd75ad09200ded81be6619fff6a3/lib/dynamodb/converter.js#L16-L21
But I'm not sure where to pass in that flag.
Also, we may also have to avoid using Number() to parse number types in the following locations:
Line 389 in 56b4615
Line 430 in 56b4615
Line 121 in 56b4615
something like req.query[key] = req.query[key].replace(/\D/g,''); is probably suitable in those cases.
From some quick testing, this seems to be a limitation of aws-sdk on the backend side.
Version 3 of the SDK fixes this (confirmed via testing), however it's still in developer preview (for a year now) and it will change the way in which you interact with items.
See an example here:
Note that attribute types are now part of item and must be dealt with explicitly. I personally think this is good, but some might disagree and argue that it adds overhead for a development tool :)
@rhl do you have any thoughts here? I can send a PR to upgrade to the new SDK if you're on board
I would be OK with updating to version 3 of SDK if it retains full compatibility with existing databases and won't modify them in some way that would be incompatible with version 2.
But if there is a converter we can use, maybe that's safer and less work.
@rchl I don't expect that the new SDK version would break compatibility with any existing DBs, just that it'll change the usage of dynamodb-admin slightly in the sense that you'll see type metadata on the edit item page
It may be possible to intercept the deserialization occurring in v2 of the SDK, but this would be hacky at best and probably not a great long-term solution.
I created #83 as a proof-of-concept if you want to play around. There's a lot of left to do before it would be ready to merge, but you can at least create a table and get a feel for the usability changes
This is causing pain for me as well.