From f0db3ec76f0793a6c1a925a275cda804eb84994a Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Wed, 22 May 2024 17:07:06 -0400 Subject: [PATCH 1/4] fix: return extra args when autoPaginate is on --- package.json | 2 +- src/bigquery.ts | 17 +++++++---- src/job.ts | 4 ++- system-test/bigquery.ts | 28 ++++++++++++++++++ test/bigquery.ts | 24 +++++++++------- tpcTest.js | 63 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 120 insertions(+), 18 deletions(-) create mode 100644 tpcTest.js diff --git a/package.json b/package.json index c75e3130..8f4029c8 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ }, "dependencies": { "@google-cloud/common": "^5.0.0", - "@google-cloud/paginator": "^5.0.0", + "@google-cloud/paginator": "^5.0.1", "@google-cloud/precise-date": "^4.0.0", "@google-cloud/promisify": "^4.0.0", "arrify": "^2.0.1", diff --git a/src/bigquery.ts b/src/bigquery.ts index 9a4b8fff..aee1485c 100644 --- a/src/bigquery.ts +++ b/src/bigquery.ts @@ -80,15 +80,18 @@ export type PagedRequest

= P & { maxApiCalls?: number; }; +export type QueryResultsResponse = bigquery.IGetQueryResultsResponse & + bigquery.IQueryResponse; + export type QueryRowsResponse = PagedResponse< RowMetadata, Query, - bigquery.IGetQueryResultsResponse + QueryResultsResponse >; export type QueryRowsCallback = PagedCallback< RowMetadata, Query, - bigquery.IGetQueryResultsResponse + QueryResultsResponse >; export type SimpleQueryRowsResponse = [RowMetadata[], bigquery.IJob]; @@ -165,8 +168,8 @@ export type GetJobsCallback = PagedCallback< bigquery.IJobList >; -export type JobsQueryResponse = [Job, bigquery.IQueryResponse]; -export type JobsQueryCallback = ResourceCallback; +export type JobsQueryResponse = [Job, QueryResultsResponse]; +export type JobsQueryCallback = ResourceCallback; export interface BigQueryTimeOptions { hours?: number | string; @@ -2113,7 +2116,10 @@ export class BigQuery extends Service { * ``` */ query(query: string, options?: QueryOptions): Promise; - query(query: Query, options?: QueryOptions): Promise; + query( + query: Query, + options?: QueryOptions + ): Promise | Promise; query( query: string, options: QueryOptions, @@ -2185,6 +2191,7 @@ export class BigQuery extends Service { } this.trace_('[runJobsQuery] job complete'); options._cachedRows = rows; + options._cachedResponse = res; if (res.pageToken) { this.trace_('[runJobsQuery] has more pages'); options.pageToken = res.pageToken; diff --git a/src/job.ts b/src/job.ts index a950d1ed..78e6a6d2 100644 --- a/src/job.ts +++ b/src/job.ts @@ -56,6 +56,7 @@ export type QueryResultsOptions = { * internal properties */ _cachedRows?: any[]; + _cachedResponse?: bigquery.IQueryResponse; }; /** @@ -571,8 +572,9 @@ class Job extends Operation { pageToken: options.pageToken, }); delete nextQuery._cachedRows; + delete nextQuery._cachedResponse; } - callback!(null, options._cachedRows, nextQuery); + callback!(null, options._cachedRows, nextQuery, options._cachedResponse); return; } diff --git a/system-test/bigquery.ts b/system-test/bigquery.ts index 0e6b9253..1bebd22a 100644 --- a/system-test/bigquery.ts +++ b/system-test/bigquery.ts @@ -34,7 +34,9 @@ import { Routine, Table, InsertRowsStreamResponse, + Query, } from '../src'; +import bq from '../src/types'; const bigquery = new BigQuery(); const storage = new Storage(); @@ -341,6 +343,32 @@ describe('BigQuery', () => { }); }); + it('should query with jobs.query and return all PagedResponse as positional parameters', async () => { + const [rows, q, response] = await bigquery.query(query); + const res: bq.IQueryResponse = response!; + assert.strictEqual(rows!.length, 100); + assert.notEqual(q?.job?.id, undefined); + assert.notEqual(res, undefined); + assert.strictEqual(res.kind, 'bigquery#queryResponse'); + assert.notEqual(res.queryId, undefined); + assert.strictEqual(res.totalRows, '100'); + }); + + it('should query withouth jobs.query and return all PagedResponse as positional parameters', async () => { + // force getQueryResult instead of fast query path + const jobId = generateName('job'); + const [rows, q, response] = await bigquery.query({ + query, + jobId, + }); + const res: bq.IGetQueryResultsResponse = response!; + assert.strictEqual(rows!.length, 100); + assert.strictEqual((q as Query).job?.id, jobId); + assert.notEqual(res, undefined); + assert.strictEqual(res.kind, 'bigquery#getQueryResultsResponse'); + assert.strictEqual(res.totalRows, '100'); + }); + it('should allow querying in series', done => { bigquery.query( query, diff --git a/test/bigquery.ts b/test/bigquery.ts index 1c40684e..a6ae268c 100644 --- a/test/bigquery.ts +++ b/test/bigquery.ts @@ -3152,24 +3152,26 @@ describe('BigQuery', () => { }); }); - it('should call job#getQueryResults with cached rows from jobs.query', done => { + it('should call job#getQueryResults with cached rows and response from jobs.query', done => { const fakeJob = { getQueryResults: (options: QueryResultsOptions, callback: Function) => { - callback(null, options._cachedRows, FAKE_RESPONSE); + callback(null, options._cachedRows, null, options._cachedResponse); }, }; + const fakeResponse = { + jobComplete: true, + schema: { + fields: [{name: 'value', type: 'INT64'}], + }, + rows: [{f: [{v: 1}]}, {f: [{v: 2}]}, {f: [{v: 3}]}], + }; + bq.runJobsQuery = (query: {}, callback: Function) => { - callback(null, fakeJob, { - jobComplete: true, - schema: { - fields: [{name: 'value', type: 'INT64'}], - }, - rows: [{f: [{v: 1}]}, {f: [{v: 2}]}, {f: [{v: 3}]}], - }); + callback(null, fakeJob, fakeResponse); }; - bq.query(QUERY_STRING, (err: Error, rows: {}, resp: {}) => { + bq.query(QUERY_STRING, (err: Error, rows: {}, query: {}, resp: {}) => { assert.ifError(err); assert.deepStrictEqual(rows, [ { @@ -3182,7 +3184,7 @@ describe('BigQuery', () => { value: 3, }, ]); - assert.strictEqual(resp, FAKE_RESPONSE); + assert.strictEqual(resp, fakeResponse); done(); }); }); diff --git a/tpcTest.js b/tpcTest.js new file mode 100644 index 00000000..2118b9f8 --- /dev/null +++ b/tpcTest.js @@ -0,0 +1,63 @@ +// Copyright 2017 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +'use strict'; + +const {GoogleAuth, OAuth2Client} = require('google-auth-library'); + +function main(datasetName) { + // [START bigquery_tpc_quickstart] + // Imports the Google Cloud client library + const {BigQuery} = require('./build/src'); + + async function testTPC() { + // Creates a client + //const projectId = 'google-tpc-testing-environment:cloudsdk-test-project'; + const projectId = 'google-tpc-testing-environment:cloudsdk-wrong-project'; + const universeDomain = 'apis-tpclp.goog'; + const location = 'tpcl-us-central13'; + + const bigqueryClient = new BigQuery({ + location, + universeDomain, + projectId, + }); + + const universe = await bigqueryClient.authClient.getUniverseDomain(); + console.log('Universe from auth client', universe); + + const [dataset] = await bigqueryClient.createDataset(datasetName); + console.log(`Dataset ${dataset.id} created.`); + + const [datasets] = await bigqueryClient.getDatasets(); + console.log('Datasets:'); + datasets.forEach(dataset => console.log(dataset.id)); + + const ds = await bigqueryClient.dataset(datasetName); + await ds.delete(); + console.log(`Dataset ${datasetName} deleted.`); + + const [rows] = await bigqueryClient.query({ + query: 'SELECT SESSION_USER() whoami', + }); + console.log('Rows:'); + rows.forEach(row => console.log(row)); + } + + testTPC(); + // [END bigquery_tpc_quickstart] +} + +const args = process.argv.slice(2); +main(...args); From d4f67234a9dd97ea2539537de5e89fc937698fcb Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Wed, 22 May 2024 17:08:49 -0400 Subject: [PATCH 2/4] fix: remove tpc test --- tpcTest.js | 63 ------------------------------------------------------ 1 file changed, 63 deletions(-) delete mode 100644 tpcTest.js diff --git a/tpcTest.js b/tpcTest.js deleted file mode 100644 index 2118b9f8..00000000 --- a/tpcTest.js +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright 2017 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -'use strict'; - -const {GoogleAuth, OAuth2Client} = require('google-auth-library'); - -function main(datasetName) { - // [START bigquery_tpc_quickstart] - // Imports the Google Cloud client library - const {BigQuery} = require('./build/src'); - - async function testTPC() { - // Creates a client - //const projectId = 'google-tpc-testing-environment:cloudsdk-test-project'; - const projectId = 'google-tpc-testing-environment:cloudsdk-wrong-project'; - const universeDomain = 'apis-tpclp.goog'; - const location = 'tpcl-us-central13'; - - const bigqueryClient = new BigQuery({ - location, - universeDomain, - projectId, - }); - - const universe = await bigqueryClient.authClient.getUniverseDomain(); - console.log('Universe from auth client', universe); - - const [dataset] = await bigqueryClient.createDataset(datasetName); - console.log(`Dataset ${dataset.id} created.`); - - const [datasets] = await bigqueryClient.getDatasets(); - console.log('Datasets:'); - datasets.forEach(dataset => console.log(dataset.id)); - - const ds = await bigqueryClient.dataset(datasetName); - await ds.delete(); - console.log(`Dataset ${datasetName} deleted.`); - - const [rows] = await bigqueryClient.query({ - query: 'SELECT SESSION_USER() whoami', - }); - console.log('Rows:'); - rows.forEach(row => console.log(row)); - } - - testTPC(); - // [END bigquery_tpc_quickstart] -} - -const args = process.argv.slice(2); -main(...args); From add28236b4489f5cd269badfd20b6c9142daca57 Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Thu, 23 May 2024 11:51:14 -0400 Subject: [PATCH 3/4] fix: use latest version of paginator --- package.json | 2 +- system-test/bigquery.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 8f4029c8..8a4e3fda 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ }, "dependencies": { "@google-cloud/common": "^5.0.0", - "@google-cloud/paginator": "^5.0.1", + "@google-cloud/paginator": "^5.0.2", "@google-cloud/precise-date": "^4.0.0", "@google-cloud/promisify": "^4.0.0", "arrify": "^2.0.1", diff --git a/system-test/bigquery.ts b/system-test/bigquery.ts index 1bebd22a..5b9989e5 100644 --- a/system-test/bigquery.ts +++ b/system-test/bigquery.ts @@ -354,7 +354,7 @@ describe('BigQuery', () => { assert.strictEqual(res.totalRows, '100'); }); - it('should query withouth jobs.query and return all PagedResponse as positional parameters', async () => { + it('should query without jobs.query and return all PagedResponse as positional parameters', async () => { // force getQueryResult instead of fast query path const jobId = generateName('job'); const [rows, q, response] = await bigquery.query({ From bef52621dc81d104b29a5b1add0f15cf66ed2cd0 Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Fri, 24 May 2024 15:06:03 -0400 Subject: [PATCH 4/4] fix: adjust type changes to be more accurate --- src/bigquery.ts | 14 ++++++-------- system-test/bigquery.ts | 14 +++++++------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/bigquery.ts b/src/bigquery.ts index aee1485c..bc85b18d 100644 --- a/src/bigquery.ts +++ b/src/bigquery.ts @@ -80,8 +80,9 @@ export type PagedRequest

= P & { maxApiCalls?: number; }; -export type QueryResultsResponse = bigquery.IGetQueryResultsResponse & - bigquery.IQueryResponse; +export type QueryResultsResponse = + | bigquery.IGetQueryResultsResponse + | bigquery.IQueryResponse; export type QueryRowsResponse = PagedResponse< RowMetadata, @@ -168,8 +169,8 @@ export type GetJobsCallback = PagedCallback< bigquery.IJobList >; -export type JobsQueryResponse = [Job, QueryResultsResponse]; -export type JobsQueryCallback = ResourceCallback; +export type JobsQueryResponse = [Job, bigquery.IQueryResponse]; +export type JobsQueryCallback = ResourceCallback; export interface BigQueryTimeOptions { hours?: number | string; @@ -2116,10 +2117,7 @@ export class BigQuery extends Service { * ``` */ query(query: string, options?: QueryOptions): Promise; - query( - query: Query, - options?: QueryOptions - ): Promise | Promise; + query(query: Query, options?: QueryOptions): Promise; query( query: string, options: QueryOptions, diff --git a/system-test/bigquery.ts b/system-test/bigquery.ts index 5b9989e5..971c0ed5 100644 --- a/system-test/bigquery.ts +++ b/system-test/bigquery.ts @@ -34,7 +34,7 @@ import { Routine, Table, InsertRowsStreamResponse, - Query, + QueryOptions, } from '../src'; import bq from '../src/types'; @@ -355,15 +355,15 @@ describe('BigQuery', () => { }); it('should query without jobs.query and return all PagedResponse as positional parameters', async () => { - // force getQueryResult instead of fast query path + // force jobs.getQueryResult instead of fast query path const jobId = generateName('job'); - const [rows, q, response] = await bigquery.query({ - query, - jobId, - }); + const qOpts: QueryOptions = { + job: bigquery.job(jobId), + }; + const [rows, q, response] = await bigquery.query(query, qOpts); const res: bq.IGetQueryResultsResponse = response!; assert.strictEqual(rows!.length, 100); - assert.strictEqual((q as Query).job?.id, jobId); + assert.strictEqual(q?.job?.id, jobId); assert.notEqual(res, undefined); assert.strictEqual(res.kind, 'bigquery#getQueryResultsResponse'); assert.strictEqual(res.totalRows, '100');