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

Inconsistent processing of large numbers from other languages. #147

Closed
huysamen opened this issue Jul 26, 2018 · 5 comments · Fixed by #516
Closed

Inconsistent processing of large numbers from other languages. #147

huysamen opened this issue Jul 26, 2018 · 5 comments · Fixed by #516
Assignees
Labels
api: datastore Issues related to the googleapis/nodejs-datastore API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@huysamen
Copy link

Because javascript cannot process large numbers, it becomes extremely difficult to work with integrated systems.

We have Java applications storing large long values to Datastore, which now has a type Integer in Datastore. These are then unable to be read by systems using node.

While the node version can also STORE large values by using the datastore.int() method, when reading the value out of the database, it is automatically read as a number javascript type, and therefore truncated.

@JustinBeckwith JustinBeckwith added the triage me I really want to be triaged. label Jul 26, 2018
@stephenplusplus stephenplusplus added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed triage me I really want to be triaged. labels Jul 30, 2018
@stephenplusplus
Copy link
Contributor

True, we currently return raw Number values for integer and double values. (Lines for reference:

case 'doubleValue': {
return parseFloat(value, 10);
}
case 'integerValue': {
return parseInt(value, 10);
}
)

We should probably return our custom types, Int and Double. The downside is that the majority of the time, the user will want the parsed Number, as their values aren't out of bounds. However, we don't have any support for when a value is out of bounds, which isn't too good.

@JustinBeckwith any thoughts on if we should make this breaking change? I believe breaking changes don't scare us anymore?

@huysamen
Copy link
Author

huysamen commented Jul 30, 2018

Yeah I have already published a npm package just for us that uses the long npm library, but using the Datastore “native” Int and Double objects would be preferable, if they support math operators (which I dont think they do). Otherwise I would say go with long for Int and maybe big-decimal or something for Double.

But from our side, break away with the changes! Claim it in the name of progress ;)

@callmehiphop
Copy link
Contributor

I kind of think we should take a similar approach to whats being suggested in googleapis/nodejs-bigquery#6, essentially allowing a user to specify custom types to cast to. This has the added benefit of not being breaking.

@stephenplusplus @bcoe @JustinBeckwith thoughts?

@bcoe
Copy link
Contributor

bcoe commented Jun 12, 2019

@callmehiphop I very much like the idea of a consistent approach between the two libraries 👍

@JustinBeckwith
Copy link
Contributor

@callmehiphop could I ask you take this one on?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the googleapis/nodejs-datastore API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
7 participants