Skip to content

Commit b3217a3

Browse files
danieljbrucedanieljbruce
authored
feat: support high precision timestamp strings on getRows calls (#1596)
## Description This PR achieves the same objectives as https://togithub.com/googleapis/java-bigquery/pull/4010, but for Node as requested in [the planning sheet](https://docs.google.com/spreadsheets/d/1cHAw6cV8k_s40hw28C8STtlymu3MXEvTY6PaKgdgSGE/edit?resourcekey=0-hPyTawvew5ugN037XCfZXg&gid=0#gid=0). The idea is that for reading rows, the users should be able to access high precision values for timestamps if high precision values are being stored on the backend. ## Impact This PR follows a test driven development approach against the getRows method. Tests were written to evaluate what happens when getRows receives various input values for timestampOutputFormat and useInt64Timestamp. These tests revealed that not only are high precision values not being delivered to users for ISO8601_STRING return types, but also other bugs exist like calls hang on getRows calls that fail and conversion logic throughs errors for some calls that fetch rows. This PR fixes all the bugs and ensures the values with the right precision are delivered to users. This chart details the before code changes / after code changes results with impact highlighted in green: <img width="1575" height="383" alt="image" src="https://togithub.com/user-attachments/assets/75c1ff70-073c-477e-9363-8eff5be0aa5f" /> The highlighted green impact shows that conversion logic has been updated to avoid the 'cannot convert' errors and with this new logic changes are also applied to the BigQueryTimestamp class to maintain high precision on timestamp values returned to users. ## Testing New system tests to capture all useInt64Timestamp/timestampOutputFormat combinations for getRows calls.
1 parent 7b4643f commit b3217a3

4 files changed

Lines changed: 300 additions & 21 deletions

File tree

‎handwritten/bigquery/src/bigquery.ts‎

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,9 @@ export class BigQuery extends Service {
596596
wrapIntegers: boolean | IntegerTypeCastOptions;
597597
selectedFields?: string[];
598598
parseJSON?: boolean;
599+
listParams?:
600+
| bigquery.tabledata.IListParams
601+
| bigquery.jobs.IGetQueryResultsParams;
599602
},
600603
) {
601604
// deep copy schema fields to avoid mutation
@@ -2471,6 +2474,9 @@ function convertSchemaFieldValue(
24712474
wrapIntegers: boolean | IntegerTypeCastOptions;
24722475
selectedFields?: string[];
24732476
parseJSON?: boolean;
2477+
listParams?:
2478+
| bigquery.tabledata.IListParams
2479+
| bigquery.jobs.IGetQueryResultsParams;
24742480
},
24752481
) {
24762482
if (value === null) {
@@ -2530,9 +2536,43 @@ function convertSchemaFieldValue(
25302536
break;
25312537
}
25322538
case 'TIMESTAMP': {
2533-
const pd = new PreciseDate();
2534-
pd.setFullTime(PreciseDate.parseFull(BigInt(value) * BigInt(1000)));
2535-
value = BigQuery.timestamp(pd);
2539+
/*
2540+
At this point, 'value' will equal the timestamp value returned from the
2541+
server. We need to parse this value differently depending on its format.
2542+
For example, value could be any of the following:
2543+
1672574400123456
2544+
1672574400.123456
2545+
2023-01-01T12:00:00.123456789123Z
2546+
*/
2547+
const listParams = options.listParams;
2548+
const timestampOutputFormat = listParams
2549+
? listParams['formatOptions.timestampOutputFormat']
2550+
: undefined;
2551+
const useInt64Timestamp = listParams
2552+
? listParams['formatOptions.useInt64Timestamp']
2553+
: undefined;
2554+
if (timestampOutputFormat === 'ISO8601_STRING') {
2555+
// value is ISO string, create BigQueryTimestamp wrapping the string
2556+
value = BigQuery.timestamp(value);
2557+
} else if (
2558+
useInt64Timestamp !== true &&
2559+
timestampOutputFormat !== 'INT64' &&
2560+
(useInt64Timestamp !== undefined || timestampOutputFormat !== undefined)
2561+
) {
2562+
// NOTE: The additional
2563+
// (useInt64Timestamp !== undefined || timestampOutputFormat !== und...)
2564+
// check is to ensure that calls to the /query endpoint remain
2565+
// unaffected as they will not be providing any listParams.
2566+
//
2567+
// If the program reaches this point in time then
2568+
// value is float seconds so convert to BigQueryTimestamp
2569+
value = BigQuery.timestamp(Number(value));
2570+
} else {
2571+
// Expect int64 micros (default or explicit INT64)
2572+
const pd = new PreciseDate();
2573+
pd.setFullTime(PreciseDate.parseFull(BigInt(value) * BigInt(1000)));
2574+
value = BigQuery.timestamp(pd);
2575+
}
25362576
break;
25372577
}
25382578
case 'GEOGRAPHY': {
@@ -2727,6 +2767,16 @@ export class BigQueryTimestamp {
27272767
} else if (typeof value === 'string') {
27282768
if (/^\d{4}-\d{1,2}-\d{1,2}/.test(value)) {
27292769
pd = new PreciseDate(value);
2770+
if (value.match(/\.\d{10,}/) && !Number.isNaN(pd.getTime())) {
2771+
/*
2772+
TODO:
2773+
When https://github.com/googleapis/nodejs-precise-date/pull/302
2774+
is released and we have full support for picoseconds in PreciseData
2775+
then we can remove this if block.
2776+
*/
2777+
this.value = value;
2778+
return;
2779+
}
27302780
} else {
27312781
const floatValue = Number.parseFloat(value);
27322782
if (!Number.isNaN(floatValue)) {

‎handwritten/bigquery/src/table.ts‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import {JobMetadata, JobOptions} from './job';
5555
import bigquery from './types';
5656
import {IntegerTypeCastOptions} from './bigquery';
5757
import {RowQueue} from './rowQueue';
58+
import IDataFormatOptions = bigquery.IDataFormatOptions;
5859

5960
// This is supposed to be a @google-cloud/storage `File` type. The storage npm
6061
// module includes these types, but is current installed as a devDependency.
@@ -1865,21 +1866,34 @@ class Table extends ServiceObject {
18651866
callback!(err, null, null, resp);
18661867
return;
18671868
}
1868-
rows = BigQuery.mergeSchemaWithRows_(this.metadata.schema, rows || [], {
1869-
wrapIntegers,
1870-
selectedFields,
1871-
parseJSON,
1872-
});
1869+
try {
1870+
/*
1871+
Without this try/catch block, calls to getRows will hang indefinitely if
1872+
a call to mergeSchemaWithRows_ fails because the error never makes it to
1873+
the callback. Instead, pass the error to the callback the user provides
1874+
so that the user can see the error.
1875+
*/
1876+
rows = BigQuery.mergeSchemaWithRows_(this.metadata.schema, rows || [], {
1877+
wrapIntegers,
1878+
selectedFields,
1879+
parseJSON,
1880+
listParams: qs,
1881+
});
1882+
} catch (err) {
1883+
callback!(err as Error | null, null, null, resp);
1884+
return;
1885+
}
18731886
callback!(null, rows, nextQuery, resp);
18741887
};
1875-
1876-
const qs = extend(
1877-
{
1878-
'formatOptions.useInt64Timestamp': true,
1879-
},
1880-
options,
1881-
);
1882-
1888+
const hasAnyFormatOpts =
1889+
options['formatOptions.timestampOutputFormat'] !== undefined ||
1890+
options['formatOptions.useInt64Timestamp'] !== undefined;
1891+
const defaultOpts = hasAnyFormatOpts
1892+
? {}
1893+
: {
1894+
'formatOptions.timestampOutputFormat': 'ISO8601_STRING',
1895+
};
1896+
const qs = extend(defaultOpts, options);
18831897
this.request(
18841898
{
18851899
uri: '/data',

‎handwritten/bigquery/system-test/timestamp_output_format.ts‎

Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,216 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import * as assert from 'assert';
16+
import {describe, it, before, after} from 'mocha';
17+
import {BigQuery} from '../src/bigquery';
18+
import {randomUUID} from 'crypto';
19+
import {RequestResponse} from '@google-cloud/common/build/src/service-object';
20+
21+
const bigquery = new BigQuery();
22+
23+
interface TestCase {
24+
name: string;
25+
timestampOutputFormat?: string;
26+
useInt64Timestamp?: boolean;
27+
expectedError?: string;
28+
expectedTsValue?: string;
29+
}
30+
31+
describe('Timestamp Output Format System Tests', () => {
32+
const datasetId = `timestamp_test_${randomUUID().replace(/-/g, '_')}`;
33+
const tableId = `timestamp_table_${randomUUID().replace(/-/g, '_')}`;
34+
const dataset = bigquery.dataset(datasetId);
35+
const table = dataset.table(tableId);
36+
const insertedTsValue = '2023-01-01T12:00:00.123456789123Z';
37+
const expectedTsValueMicroseconds = '2023-01-01T12:00:00.123456000Z';
38+
const expectedTsValueNanoseconds = '2023-01-01T12:00:00.123456789123Z';
39+
40+
before(async () => {
41+
await dataset.create();
42+
await table.create({
43+
schema: [{name: 'ts', type: 'TIMESTAMP', timestampPrecision: '12'}],
44+
});
45+
// Insert a row to test retrieval
46+
await table.insert([{ts: insertedTsValue}]);
47+
});
48+
49+
after(async () => {
50+
try {
51+
await dataset.delete({force: true});
52+
} catch (e) {
53+
console.error('Error deleting dataset:', e);
54+
}
55+
});
56+
57+
const testCases: TestCase[] = [
58+
{
59+
name: 'should call getRows with TIMESTAMP_OUTPUT_FORMAT_UNSPECIFIED and useInt64Timestamp=true',
60+
timestampOutputFormat: 'TIMESTAMP_OUTPUT_FORMAT_UNSPECIFIED',
61+
useInt64Timestamp: true,
62+
expectedTsValue: expectedTsValueMicroseconds,
63+
},
64+
{
65+
name: 'should call getRows with TIMESTAMP_OUTPUT_FORMAT_UNSPECIFIED and useInt64Timestamp=false',
66+
timestampOutputFormat: 'TIMESTAMP_OUTPUT_FORMAT_UNSPECIFIED',
67+
useInt64Timestamp: false,
68+
expectedTsValue: expectedTsValueMicroseconds,
69+
},
70+
{
71+
name: 'should call getRows with FLOAT64 and useInt64Timestamp=true (expect error)',
72+
timestampOutputFormat: 'FLOAT64',
73+
useInt64Timestamp: true,
74+
expectedError:
75+
'Cannot specify both use_int64_timestamp and timestamp_output_format.',
76+
},
77+
{
78+
name: 'should call getRows with FLOAT64 and useInt64Timestamp=false',
79+
timestampOutputFormat: 'FLOAT64',
80+
useInt64Timestamp: false,
81+
expectedTsValue: expectedTsValueMicroseconds,
82+
},
83+
{
84+
name: 'should call getRows with INT64 and useInt64Timestamp=true',
85+
timestampOutputFormat: 'INT64',
86+
useInt64Timestamp: true,
87+
expectedTsValue: expectedTsValueMicroseconds,
88+
},
89+
{
90+
name: 'should call getRows with INT64 and useInt64Timestamp=false',
91+
timestampOutputFormat: 'INT64',
92+
useInt64Timestamp: false,
93+
expectedTsValue: expectedTsValueMicroseconds,
94+
},
95+
{
96+
name: 'should call getRows with ISO8601_STRING and useInt64Timestamp=true (expect error)',
97+
timestampOutputFormat: 'ISO8601_STRING',
98+
useInt64Timestamp: true,
99+
expectedError:
100+
'Cannot specify both use_int64_timestamp and timestamp_output_format.',
101+
},
102+
{
103+
name: 'should call getRows with ISO8601_STRING and useInt64Timestamp=false',
104+
timestampOutputFormat: 'ISO8601_STRING',
105+
useInt64Timestamp: false,
106+
expectedTsValue: expectedTsValueNanoseconds,
107+
},
108+
// Additional test cases for undefined combinations
109+
{
110+
name: 'should call getRows with timestampOutputFormat undefined and useInt64Timestamp undefined',
111+
timestampOutputFormat: undefined,
112+
useInt64Timestamp: undefined,
113+
expectedTsValue: expectedTsValueNanoseconds,
114+
},
115+
{
116+
name: 'should call getRows with timestampOutputFormat undefined and useInt64Timestamp=true',
117+
timestampOutputFormat: undefined,
118+
useInt64Timestamp: true,
119+
expectedTsValue: expectedTsValueMicroseconds,
120+
},
121+
{
122+
name: 'should call getRows with timestampOutputFormat undefined and useInt64Timestamp=false',
123+
timestampOutputFormat: undefined,
124+
useInt64Timestamp: false,
125+
expectedTsValue: expectedTsValueMicroseconds,
126+
},
127+
{
128+
name: 'should call getRows with TIMESTAMP_OUTPUT_FORMAT_UNSPECIFIED and useInt64Timestamp undefined',
129+
timestampOutputFormat: 'TIMESTAMP_OUTPUT_FORMAT_UNSPECIFIED',
130+
useInt64Timestamp: undefined,
131+
expectedTsValue: expectedTsValueMicroseconds,
132+
},
133+
{
134+
name: 'should call getRows with FLOAT64 and useInt64Timestamp undefined (expect error)',
135+
timestampOutputFormat: 'FLOAT64',
136+
useInt64Timestamp: undefined,
137+
expectedTsValue: expectedTsValueMicroseconds,
138+
},
139+
{
140+
name: 'should call getRows with INT64 and useInt64Timestamp undefined',
141+
timestampOutputFormat: 'INT64',
142+
useInt64Timestamp: undefined,
143+
expectedTsValue: expectedTsValueMicroseconds,
144+
},
145+
{
146+
name: 'should call getRows with ISO8601_STRING and useInt64Timestamp undefined (expect error)',
147+
timestampOutputFormat: 'ISO8601_STRING',
148+
useInt64Timestamp: undefined,
149+
expectedTsValue: expectedTsValueNanoseconds,
150+
},
151+
];
152+
153+
testCases.forEach(
154+
({
155+
name,
156+
timestampOutputFormat,
157+
useInt64Timestamp,
158+
expectedError,
159+
expectedTsValue,
160+
}) => {
161+
it(name, async () => {
162+
const options: {[key: string]: any} = {};
163+
if (timestampOutputFormat !== undefined) {
164+
options['formatOptions.timestampOutputFormat'] =
165+
timestampOutputFormat;
166+
}
167+
if (useInt64Timestamp !== undefined) {
168+
options['formatOptions.useInt64Timestamp'] = useInt64Timestamp;
169+
}
170+
171+
if (expectedError) {
172+
try {
173+
await table.getRows(options);
174+
assert.fail('The call should have thrown an error.');
175+
} catch (e) {
176+
assert.strictEqual((e as Error).message, expectedError);
177+
}
178+
} else {
179+
const [rows] = await table.getRows(options);
180+
assert(rows.length > 0);
181+
assert.strictEqual(rows[0].ts.value, expectedTsValue);
182+
}
183+
});
184+
},
185+
);
186+
187+
it('should make a request with ISO8601_STRING when no format options are being used', done => {
188+
void (async () => {
189+
const originalRequest = table.request;
190+
const requestPromise: Promise<RequestResponse> = new Promise(
191+
(resolve, reject) => {
192+
const innerPromise = new Promise((innerResolve, innerReject) => {
193+
innerResolve({});
194+
});
195+
resolve(innerPromise as Promise<RequestResponse>);
196+
},
197+
);
198+
table.request = reqOpts => {
199+
table.request = originalRequest;
200+
if (
201+
reqOpts.qs['formatOptions.timestampOutputFormat'] === 'ISO8601_STRING'
202+
) {
203+
done();
204+
} else {
205+
done(
206+
new Error(
207+
'The default timestampOutputFormat should be ISO8601_STRING',
208+
),
209+
);
210+
}
211+
return requestPromise;
212+
};
213+
await table.getRows({});
214+
})();
215+
});
216+
});

0 commit comments

Comments
 (0)