googleapis/nodejs-bigquery

Library cannot parse timestamp value "0" - Cannot convert a BigInt value to a number as PreciseDate

guisalim opened this issue · 7 comments

guisalim commented

Environment details

  • OS: OSx
  • Node.js version: 20.12.1
  • npm version: 10.5.0
  • @google-cloud/bigquery version: 7.5.2

Steps to reproduce

  1. For a given row in a BQ table, have a timestamp column value of 1970-01-01 00:00:00 UTC (aka timestamp 0)
  2. Run a query + getQueryResults() to fetch the row
 const [job] = await createQueryJob({ query: "SELECT * FROM .... " });
 const [result] = await job.getQueryResults(); // ---- Error: TypeError: Cannot convert a BigInt value to a number at PreciseDate

Here's the stack trace of the error:

at new Date (<anonymous>)
    at new PreciseDate (<{PATH}>/node_modules/@google-cloud/precise-date/build/src/index.js:95:26)
    at new BigQueryTimestamp (<{PATH}>/node_modules/@google-cloud/bigquery/build/src/bigquery.js:1339:22)
    at Function.timestamp (<{PATH}>/node_modules/@google-cloud/bigquery/build/src/bigquery.js:626:16)
    at convert (<{PATH}>/node_modules/@google-cloud/bigquery/build/src/bigquery.js:374:38)
    at <{PATH}>/node_modules/@google-cloud/bigquery/build/src/bigquery.js:310:29
    at Array.map (<anonymous>)
    at mergeSchema (<{PATH}>/node_modules/@google-cloud/bigquery/build/src/bigquery.js:301:26)
    at Array.map (<anonymous>)
    at Function.mergeSchemaWithRows_ (<{PATH}>/node_modules/@google-cloud/bigquery/build/src/bigquery.js:299:29)
    at <{PATH}>/node_modules/@google-cloud/bigquery/build/src/job.js:368:44
    at <{PATH}>/node_modules/@google-cloud/common/build/src/util.js:412:25
    at Util.handleResp (<{PATH}>/node_modules/@google-cloud/common/build/src/util.js:161:9)
    at <{PATH}>/node_modules/@google-cloud/common/build/src/util.js:534:22
    at onResponse (<{PATH}>/node_modules/retry-request/index.js:259:7)
    at <{PATH}>/node_modules/teeny-request/src/index.ts:333:11
    at processTicksAndRejections (node:internal/process/task_queues:95:5)'

After investigating the issue, I noticed that the following statement is causing the error (class BigQueryTimestamp > constructor):

// @google-cloud/bigquery/build/src/bigquery.js:1339
     pd = new precise_date_1.PreciseDate(BigInt(value) * BigInt(1000));

If value = "0", PreciseDate. constructor will try to run new Date(BigInt("0")), which throws an error.

The issue was introduced by the client version 7.5.

Thanks!

alvarowolfx commented

the line that throws the error is this one:

nodejs-bigquery/src/bigquery.ts

Line 668 in 74aa150

const pd = new PreciseDate(BigInt(value) * BigInt(1000));
, not on the BigQueryTimestamp class. I'm investigating here, but it's a odd one, because there are tests even for negative numbers and the PreciseDate class actually supports BigInts.

guisalim commented

the line that throws the error is this one:

nodejs-bigquery/src/bigquery.ts

Line 668 in 74aa150

const pd = new PreciseDate(BigInt(value) * BigInt(1000));

, not on the BigQueryTimestamp class. I'm investigating here, but it's a odd one, because there are tests even for negative numbers and the PreciseDate class actually supports BigInts.

The issue is that when the value comes from BigQuery, it comes as a string "0" - If you parse as BigInt, it results in 0n.

// This
const pd = new PreciseDate(BigInt(value) * BigInt(1000)); 

// Same as
const pd = new PreciseDate(BigInt("0") * BigInt(1000)); // equals to `0n`

Downstream, that will result in new Date(0n) => throws the error mentioned

guisalim commented

This PR #1354 handles this issue, btw.

(feel free to suggest any change to fit it to the code architecture)

alvarowolfx commented

the line that throws the error is this one:

nodejs-bigquery/src/bigquery.ts

Line 668 in 74aa150

const pd = new PreciseDate(BigInt(value) * BigInt(1000));

, not on the BigQueryTimestamp class. I'm investigating here, but it's a odd one, because there are tests even for negative numbers and the PreciseDate class actually supports BigInts.

The issue is that when the value comes from BigQuery, it comes as a string "0" - If you parse as BigInt, it results in 0n.

// This
const pd = new PreciseDate(BigInt(value) * BigInt(1000)); 

// Same as
const pd = new PreciseDate(BigInt("0") * BigInt(1000)); // equals to `0n`

Downstream, that will result in new Date(0n) => throws the error mentioned

Yeah, the problem itself is on the PreciseDate class, there are code paths that doesn't even hit the BigQueryTimestamp that triggers the issue, so PR #1354 doesn't fully fix the issue.

This is my test code and it still fails with the PR, because the issue is on the line that I mentioned before (

nodejs-bigquery/src/bigquery.ts

Line 668 in 74aa150

const pd = new PreciseDate(BigInt(value) * BigInt(1000));
)

const sqlQuery =
    `SELECT TIMESTAMP_ADD(@ts_value, INTERVAL 1 HOUR) as ts 
     UNION ALL 
     SELECT CAST('1969-12-25 00:00:00' as TIMESTAMP) as ts
     UNION ALL 
     SELECT CAST('1970-01-01 00:00:01' as TIMESTAMP) as ts
     UNION ALL 
     SELECT CAST('1970-01-01 00:00:00' as TIMESTAMP) as ts
    `;
  const options = {
    query: sqlQuery,
    location: 'US',
    params: {ts_value: new Date()},
  };

  // Run the query
  const [rows] = await bigquery.query(options);

  console.log('Rows:');
  rows.forEach(row => console.log(row.ts));

The fix that I implemented locally here is this one:

         case 'TIMESTAMP': {
-          const pd = new PreciseDate(BigInt(value) * BigInt(1000));
+          const pd = new PreciseDate();
+          pd.setFullTime(PreciseDate.parseFull(BigInt(value) * BigInt(1000)));
           value = BigQuery.timestamp(pd);
           break;
         }

I'm adding some system-test to capture that, but I'll also later fix the root cause on the precise-date library.

  • ❤️1
alvarowolfx commented

On the precise-date library ,this line https://github.com/googleapis/nodejs-precise-date/blob/04b3784b0a9948687cbd98a2be376246589c372c/src/index.ts#L122 , needs to explicitly check for zero values.

guisalim commented

Cool 😄 Thanks @alvarowolfx!

Looking forward to having it published!

alvarowolfx commented

@guisalim unfortunately our releases are paused due to Google Cloud Next. Can you revert temporarily to a version before v7.5.0 ? I'll submit the PR and get it merged, but I'll not be able to publish the fix until after Next. Another path is to add this package as a dependency directly from Git (for reference https://docs.npmjs.com/cli/v9/configuring-npm/package-json?v=true#git-urls-as-dependencies). Let me know if any of that works for you, I want to make sure that you're not blocked in the meantime.