-
Notifications
You must be signed in to change notification settings - Fork 150
Add preferNativeInt for unpack operation #94
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
Conversation
This forces database results to be read as language-native number values unless they are out of range, which reduces the code overhead for applications that do not need to worry about the extended range.
Sorry for not getting back to you on this. We like this feature but we maybe like to support in a slightly different manner. There may be several ways a user want to affect serialization and we are discussing adding this kind of support across all drivers. That is what taking us so long to decide on this PR. |
this would be useful |
* @private | ||
*/ | ||
var MAX_NATIVE = Integer.fromValue(Number.MAX_SAFE_INTEGER); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason you have added these as variables in module scope? Perhaps you could attach them to the Integer
constructor like so:
Integer.MIN_NATIVE = Integer.fromValue(Number.MIN_SAFE_INTEGER)
Integer.MAX_NATIVE = Integer.fromValue(Number.MAX_SAFE_INTEGER)
I'd do it myself, but I'm not sure if mucking about with someone's PR is frowned upon :)
It would be nice to have these available in the same place like other MIN_MAX values, perhaps you could even reuse the others, but I don't have the driver on hand to test myself. Might try over lunch
[edit] d'oh, just saw @pontusmelke's comment...
I wrote a function that recursively maps Result objects to their Javascript native equivalents. https://github.com/codex-digital/cypher-stream/blob/master/util/to-native.js Figured it might be useful to someone. |
Hi, sorry for not coming back to this PR for a long time. We finally agreed to keep |
Is there any chance of reopening the discussion on this or a similar PR? The integer functions provided are helpful, but not a complete solution, as using them requires that you know exactly the format of the data being returned, as highlighted in this issue raised in May: #245 We're currently in the process of moving some of our apps over from the thingdom client so that they can take advantage of bolt, but this issue feels like a major disadvantage of the new client |
This forces database results to be read as language-native number values unless they are out of range, which reduces the code overhead for applications that do not need to worry about the extended range.