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

large integer IDs are rounded #82

Open
mstorus opened this issue Sep 25, 2019 · 7 comments
Open

large integer IDs are rounded #82

mstorus opened this issue Sep 25, 2019 · 7 comments

Comments

@mstorus
Copy link

mstorus commented Sep 25, 2019

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:
DynamoDB Admin 2019-09-25 15-36-22

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:

req.query[key] = Number(req.query[key])

req.query[key] = Number(req.query[key])

return Number(keyValue)

something like req.query[key] = req.query[key].replace(/\D/g,''); is probably suitable in those cases.

@emadow
Copy link

emadow commented Sep 27, 2019

👍 Also having this issue

@garrettheel
Copy link
Contributor

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:

Screen Shot 2019-10-19 at 11 19 00 AM

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

@rchl
Copy link
Collaborator

rchl commented Oct 20, 2019

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.

@garrettheel
Copy link
Contributor

@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.

@garrettheel
Copy link
Contributor

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

@rixth
Copy link

rixth commented Mar 26, 2020

This is causing pain for me as well.

@irshaad-auhammad-cko
Copy link

Has there been any new changes on this by any chance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants