-
Notifications
You must be signed in to change notification settings - Fork 55
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
fix: Normalize text protocol handling of numeric and numeric[] types #91
fix: Normalize text protocol handling of numeric and numeric[] types #91
Conversation
Normalizes handling of numeric[] types to always return a string array representation of the value for both the text. Previously numeric array types were being returned as an array of Number which could lose precision.
Closing this out as it was included in #94 |
… as one commit labelled “Add linter (standard)”, ee22252. It’s kind of hidden in there. Is it too late to split them? @bendrucker |
I haven't published a release to npm. It's been about 24h so I think it's worth it to rewrite the history to split it. Will do now. |
Normalizes handling of numeric[] types to always return a string array representation of the value for both the text. Previously numeric array types were being returned as an array of Number which could lose precision. Closes #91
Did some history rewriting to fix this. Anything else we should be doing around numbers in a breaking release here? It would be nice to give people the options to get |
Although I guess that really only relates to count/int behavior, not floats. But I'd like to still include a breaking change for #78 (enabled by default if supported) in the same major that releases this PR. |
@bendrucker Thanks for the history cleanup! I'm a -1 on feature detection changing results. If we're still going to support Node v8 until it's out of LTS then I say release one semver major with the current round of changes and another in Jan 2020 when Node v8 is EOL. For the record, I'm for the BigInt change itself, just seems brittle to have a node version change the output type of a column. Anything that uses it would have to perform it's own version detection (even the tests for this module). |
@sehrope Did you do a follow-up PR? Can't seem to find one. |
@contra Follow up for what? The text protocol fix for numeric/numeric[] was added in afead5b (it's this PR after some rewriting to fix me accidentally checking in the packed up module). @charmander has a separate branch to redo the binary parsing but it hasn't been submitted as a PR yet. |
@sehrope Thanks, exactly what I was looking for. |
to match the text parser (and the text array parser, as of brianc#91).
NOTE: This does not fix the binary protocol, only the text one.