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

Feature request: checkNumberRange: boolean #201

Closed
urugator opened this issue May 26, 2022 · 3 comments
Closed

Feature request: checkNumberRange: boolean #201

urugator opened this issue May 26, 2022 · 3 comments

Comments

@urugator
Copy link
Contributor

urugator commented May 26, 2022

BigInt as a safe default is good, but I often find it quite impractical, because mariadb returns BIGINT from various operations like COUNT(*) or col + 1 (note CAST(aBigInt AS INT) still results in BIGINT).
At the moment we can either choose to use the default safe behavior, but then we have to check the range and cast these to number manually or we can use bigIntAsNumber: true for convinience, but risk an invalid behavior in edge cases.
I propose introducing a new option, which would provide conviniency, but would fail loudly in case of possible error:
checkNumberRange: boolean
When set to true, the driver throws an error if DECIMAL/BIGINT can't be converted safely to number.
The option is only effective when one of insertIdAsNumber/decimalAsNumber/bigIntAsNumber is true.
Defaults to false for BC reasons.

@rusher
Copy link
Collaborator

rusher commented May 30, 2022

Right, i think that can be a nice addition to and a very practical option.
I've create https://jira.mariadb.org/browse/CONJS-198 for that.
This will be part of next release.

rusher added a commit that referenced this issue Jun 2, 2022
rusher added a commit that referenced this issue Jun 2, 2022
rusher added a commit that referenced this issue Jun 2, 2022
@rusher
Copy link
Collaborator

rusher commented Jul 1, 2022

closing, since done in develop branch and will be part of 3.0.1

@rusher rusher closed this as completed Jul 1, 2022
@urugator
Copy link
Contributor Author

urugator commented Aug 8, 2022

Thank you for the feature! I think you forget to add the new option to types:
https://github.com/mariadb-corporation/mariadb-connector-nodejs/blob/master/types/index.d.ts
Should I open new issue?

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

2 participants