Added support for the NUMERIC values.#119
Conversation
Codecov Report
@@ Coverage Diff @@
## master #119 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 4 4
Lines 696 704 +8
=====================================
+ Hits 696 704 +8
Continue to review full report at Codecov.
|
tswast
left a comment
tswast
left a comment
There was a problem hiding this comment.
LGTM, thanks!
I'm not familiar enough with the Node.js ecosystem to know if big.js is the right choice, but it does appear to have some healthy activity on the repo. https://github.com/MikeMcl/big.js
Note to others reviewing: this PR was authored by a Googler on the BigQuery team.
|
Good question for @googleapis/node-team :) |
|
What's the forward compatibility story with the core BigInt language feature that we just shipped with Node 10? |
ofrobots
left a comment
ofrobots
left a comment
There was a problem hiding this comment.
Blocking until forward compatibility story is understood.
|
@ofrobots Do BigInts support fractional values? |
|
It does not. I've removed the block. Will take a deeper look later. |
|
For posterity here is a link to the BigQuery docs on the NUMERIC data type: https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#numeric-type |
ofrobots
left a comment
ofrobots
left a comment
There was a problem hiding this comment.
LGTM on the choice of big.js. It seems to be a fairly healthy project with a large amount of usage. There also seems to be a corresponding types package which itself is fairly heavily used. Seems like a good choice.
@stephenplusplus to approve.
|
LGTM, but we should probably have a system test for this new type. |
|
@stephenplusplus Added a system test, please take another look. |
Reverted a change that the local linter made but the CI's one does not like.
|
Can somebody merge the changes? I do not have write access to the repo. |
|
I upgraded the bigquery node js library after a long time and my app broke because of numeric values being returned as Big() values. Is it possible for me to have the existing behaviour where I got the numeric result back as string? I am storing the query result in firestore which won't support Big() and converting the results manually to string seems tedious. Is there any query option one can use to disable this new behaviour? |
|
The string representation was a stopgap before the NUMERIC type was made
generally available. You can CAST inside the query itself if you want a
string to come back instead of Big().
…On Thu, Jan 31, 2019 at 2:43 AM nikhilag ***@***.***> wrote:
I upgraded the bigquery node js library aftera long time and my app broke
because of numeric values being returned as Big() values. Is it possible
for me to have the existing behaviour where I got the numeric result back
as string? I am storing the query result in firestore which won't support
Big() and converting the results manually to string seems tedious. Is there
any query option one can use to disable this new behaviour?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#119 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AZFHMFBukaTg6OEFcpesbfC-UuQZvyBMks5vIsjLgaJpZM4UlRQp>
.
|
7 participants
This change adds support for the newly added NUMERIC data type to the nodejs BigQuery library.