From c2a684172cd77087e836ea6fe54cb80da5bfdbe0 Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Fri, 16 Feb 2024 11:37:33 -0400 Subject: [PATCH 1/9] feat: integrate jobs.query and jobless query for faster queries --- src/bigquery.ts | 138 +++++++++++++++++++++++++++++++++++++++++++++--- src/job.ts | 23 ++++++++ src/logger.ts | 47 +++++++++++++++++ src/types.d.ts | 7 +++ 4 files changed, 207 insertions(+), 8 deletions(-) create mode 100644 src/logger.ts diff --git a/src/bigquery.ts b/src/bigquery.ts index f3e14113..e080684b 100644 --- a/src/bigquery.ts +++ b/src/bigquery.ts @@ -45,6 +45,7 @@ import { } from './table'; import {GoogleErrorBody} from '@google-cloud/common/build/src/util'; import bigquery from './types'; +import {logger, setLogFunction} from './logger'; // Third-Party Re-exports export {common}; @@ -164,6 +165,9 @@ export type GetJobsCallback = PagedCallback< bigquery.IJobList >; +export type JobsQueryResponse = [Job, bigquery.IQueryResponse]; +export type JobsQueryCallback = ResourceCallback; + export interface BigQueryTimeOptions { hours?: number | string; minutes?: number | string; @@ -488,6 +492,11 @@ export class BigQuery extends Service { }); } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private trace(msg: string, ...otherArgs: any[]) { + logger('[bigquery]', msg, ...otherArgs); + } + get universeDomain() { return this._universeDomain; } @@ -1439,6 +1448,7 @@ export class BigQuery extends Service { }, options ); + this.trace('[createQueryJob]', query); if (options.destination) { if (!(options.destination instanceof Table)) { @@ -2106,22 +2116,132 @@ export class BigQuery extends Service { : {}; const callback = typeof optionsOrCallback === 'function' ? optionsOrCallback : cb; - this.createQueryJob(query, (err, job, resp) => { + + this.trace('[query]', query, options); + const queryReq = this.probeFastPath_(query, options); + if (!queryReq) { + this.createQueryJob(query, (err, job, resp) => { + if (err) { + (callback as SimpleQueryRowsCallback)(err, null, resp); + return; + } + if (typeof query === 'object' && query.dryRun) { + (callback as SimpleQueryRowsCallback)(null, [], resp); + return; + } + // The Job is important for the `queryAsStream_` method, so a new query + // isn't created each time results are polled for. + options = extend({job}, queryOpts, options); + job!.getQueryResults(options, callback as QueryRowsCallback); + }); + return; + } + + this.syncQuery(queryReq, options, (err, job, res) => { if (err) { - (callback as SimpleQueryRowsCallback)(err, null, resp); + (callback as SimpleQueryRowsCallback)(err, null, res); return; } - if (typeof query === 'object' && query.dryRun) { - (callback as SimpleQueryRowsCallback)(null, [], resp); - return; + + let nextQuery = extend({job}, options); + if (res && res.jobComplete) { + let rows: any = []; + if (res.schema && res.rows) { + rows = BigQuery.mergeSchemaWithRows_(res.schema, res.rows, { + wrapIntegers: options.wrapIntegers!, // TODO: fix default value + parseJSON: options.parseJSON, + }); + } + this.trace('[syncQuery] job complete'); + if (res.pageToken) { + this.trace('[syncQuery] has more pages'); + nextQuery = extend({job}, options, { + pageToken: res.pageToken, + cachedRows: rows, + }); + job!.getQueryResults(nextQuery, callback as QueryRowsCallback); + return; + } else { + this.trace('[syncQuery] no more pages'); + (callback as SimpleQueryRowsCallback)(err, rows, res); + return; + } } - // The Job is important for the `queryAsStream_` method, so a new query - // isn't created each time results are polled for. - options = extend({job}, queryOpts, options); + this.trace('[syncQuery] job not complete'); job!.getQueryResults(options, callback as QueryRowsCallback); }); } + private probeFastPath_( + query: string | Query, + options: QueryOptions + ): bigquery.IQueryRequest | undefined { + this.trace('[probeFastPath_]', query, options); + if (process.env.FAST_QUERY_PATH === 'DISABLED') { + return undefined; + } + if (typeof query === 'string') { + if (!options.job) { + const req: bigquery.IQueryRequest = { + ...options, + useQueryCache: false, + jobCreationMode: 'JOB_CREATION_OPTIONAL', + useLegacySql: false, + requestId: uuid.v4(), + query: query, + }; + return req; + } + return undefined; + } + // TODO: non string query and convert to QueryRequest + return undefined; + } + + syncQuery( + req: bigquery.IQueryRequest, + options?: QueryResultsOptions + ): Promise; + syncQuery( + req: bigquery.IQueryRequest, + options: QueryResultsOptions, + callback: JobsQueryCallback + ): void; + syncQuery( + req: bigquery.IQueryRequest, + optionsOrCallback?: QueryResultsOptions | JobsQueryCallback, + cb?: JobsQueryCallback + ): void | Promise { + const options = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + const callback = + typeof optionsOrCallback === 'function' ? optionsOrCallback : cb; + + this.trace('[syncQuery]', req, options, callback); + this.request( + { + method: 'POST', + uri: '/queries', + json: req, + }, + async (err, res: bigquery.IQueryResponse) => { + if (err) { + callback!(err, null, res); + return; + } + this.trace('jobs.query res:', res); + let job: Job | null = null; + if (res.jobReference) { + const jobRef = res.jobReference!; + job = this.job(jobRef.jobId!, { + location: jobRef.location, + }); + } + callback!(null, job, res); + } + ); + } + /** * This method will be called by `createQueryStream()`. It is required to * properly set the `autoPaginate` option value. @@ -2153,6 +2273,8 @@ export class BigQuery extends Service { this.query(query, opts, callback); } + + static setLogFunction = setLogFunction; } /*! Developer Documentation diff --git a/src/job.ts b/src/job.ts index fd6a1481..44272e45 100644 --- a/src/job.ts +++ b/src/job.ts @@ -39,6 +39,7 @@ import { } from './bigquery'; import {RowMetadata} from './table'; import bigquery from './types'; +import {logger} from './logger'; export type JobMetadata = bigquery.IJob; export type JobOptions = JobRequest; @@ -50,6 +51,7 @@ export type QueryResultsOptions = { job?: Job; wrapIntegers?: boolean | IntegerTypeCastOptions; parseJSON?: boolean; + cachedRows?: any[]; } & PagedRequest; /** @@ -379,6 +381,11 @@ class Job extends Operation { ); } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private trace(msg: string, ...otherArgs: any[]) { + logger(`[job][${this.id}]`, msg, ...otherArgs); + } + /** * @callback CancelCallback * @param {?Error} err Request error, if any. @@ -536,6 +543,12 @@ class Job extends Operation { }, options ); + this.trace( + '[getQueryResults]', + this.id, + options.pageToken, + options.startIndex + ); const wrapIntegers = qs.wrapIntegers ? qs.wrapIntegers : false; delete qs.wrapIntegers; @@ -547,6 +560,15 @@ class Job extends Operation { const timeoutOverride = typeof qs.timeoutMs === 'number' ? qs.timeoutMs : false; + if (options.cachedRows) { + const nextQuery = Object.assign({}, options, { + pageToken: options.pageToken, + }); + delete nextQuery.cachedRows; + callback!(null, options.cachedRows, nextQuery); + return; + } + this.bigQuery.request( { uri: '/queries/' + this.id, @@ -582,6 +604,7 @@ class Job extends Operation { return; } } else if (resp.pageToken) { + this.trace('[getQueryResults] has more pages', resp.pageToken); // More results exist. nextQuery = Object.assign({}, options, { pageToken: resp.pageToken, diff --git a/src/logger.ts b/src/logger.ts new file mode 100644 index 00000000..c3ff69e7 --- /dev/null +++ b/src/logger.ts @@ -0,0 +1,47 @@ +// Copyright 2024 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 +// +// https://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. + +import * as util from 'util'; + +/*! The external function used to emit logs. */ +let logFunction: ((msg: string) => void) | null = null; + +/** + * Log function to use for debug output. By default, we don't perform any + * logging. + * + * @private + * @internal + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function logger(source: string, msg: string, ...otherArgs: any[]) { + if (logFunction) { + const time = new Date().toISOString(); + const formattedMsg = util.format( + `D ${time} | ${source} | ${msg} |`, + ...otherArgs + ); + logFunction(formattedMsg); + } +} + +/** + * Sets or disables the log function for all active Firestore instances. + * + * @param logger A log function that takes a message (such as `console.log`) or + * `null` to turn off logging. + */ +export function setLogFunction(logger: ((msg: string) => void) | null): void { + logFunction = logger; +} diff --git a/src/types.d.ts b/src/types.d.ts index 1f8dd6a1..236fb5eb 100644 --- a/src/types.d.ts +++ b/src/types.d.ts @@ -2984,6 +2984,13 @@ declare namespace bigquery { * [Optional] If set to true, BigQuery doesn't run the job. Instead, if the query is valid, BigQuery returns statistics about the job such as how many bytes would be processed. If the query is invalid, an error returns. The default value is false. */ dryRun?: boolean; + /** + * Optional. If not set, jobs are always required. If set, the query request will follow the behavior described JobCreationMode. This feature is not yet available. Jobs will always be created. + */ + jobCreationMode?: + | 'JOB_CREATION_MODE_UNSPECIFIED' + | 'JOB_CREATION_REQUIRED' + | 'JOB_CREATION_OPTIONAL'; /** * The resource type of the request. */ From 344c2ce62d66f1822bab2b7cd1c210bd81aa65f9 Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Thu, 22 Feb 2024 11:10:49 -0400 Subject: [PATCH 2/9] feat: rename probeFastPath method to convert queryRequests --- src/bigquery.ts | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/bigquery.ts b/src/bigquery.ts index e080684b..21d891e8 100644 --- a/src/bigquery.ts +++ b/src/bigquery.ts @@ -2118,7 +2118,7 @@ export class BigQuery extends Service { typeof optionsOrCallback === 'function' ? optionsOrCallback : cb; this.trace('[query]', query, options); - const queryReq = this.probeFastPath_(query, options); + const queryReq = this.buildQueryRequest(query, options); if (!queryReq) { this.createQueryJob(query, (err, job, resp) => { if (err) { @@ -2137,7 +2137,7 @@ export class BigQuery extends Service { return; } - this.syncQuery(queryReq, options, (err, job, res) => { + this.runJobsQuery(queryReq, options, (err, job, res) => { if (err) { (callback as SimpleQueryRowsCallback)(err, null, res); return; @@ -2152,9 +2152,9 @@ export class BigQuery extends Service { parseJSON: options.parseJSON, }); } - this.trace('[syncQuery] job complete'); + this.trace('[runJobsQuery] job complete'); if (res.pageToken) { - this.trace('[syncQuery] has more pages'); + this.trace('[runJobsQuery] has more pages'); nextQuery = extend({job}, options, { pageToken: res.pageToken, cachedRows: rows, @@ -2162,17 +2162,26 @@ export class BigQuery extends Service { job!.getQueryResults(nextQuery, callback as QueryRowsCallback); return; } else { - this.trace('[syncQuery] no more pages'); + this.trace('[runJobsQuery] no more pages'); (callback as SimpleQueryRowsCallback)(err, rows, res); return; } } - this.trace('[syncQuery] job not complete'); + this.trace('[runJobsQuery] job not complete'); job!.getQueryResults(options, callback as QueryRowsCallback); }); } - private probeFastPath_( + /** + * Check if the given Query can run using the `jobs.query` endpoint. + * Returns a bigquery.IQueryRequest that can be used to call `jobs.query`. + * Return undefined if is not possible to convert to a bigquery.IQueryRequest. + * + * @param query string | Query + * @param options QueryOptions + * @returns bigquery.IQueryRequest | undefined + */ + private buildQueryRequest( query: string | Query, options: QueryOptions ): bigquery.IQueryRequest | undefined { @@ -2198,16 +2207,16 @@ export class BigQuery extends Service { return undefined; } - syncQuery( + private runJobsQuery( req: bigquery.IQueryRequest, options?: QueryResultsOptions ): Promise; - syncQuery( + private runJobsQuery( req: bigquery.IQueryRequest, options: QueryResultsOptions, callback: JobsQueryCallback ): void; - syncQuery( + private runJobsQuery( req: bigquery.IQueryRequest, optionsOrCallback?: QueryResultsOptions | JobsQueryCallback, cb?: JobsQueryCallback @@ -2217,7 +2226,7 @@ export class BigQuery extends Service { const callback = typeof optionsOrCallback === 'function' ? optionsOrCallback : cb; - this.trace('[syncQuery]', req, options, callback); + this.trace('[runJobsQuery]', req, options, callback); this.request( { method: 'POST', From ed4381c15bdf7116dd8ba4307fa8810db420c6f5 Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Wed, 28 Feb 2024 14:28:02 -0400 Subject: [PATCH 3/9] fix: avoid promisify of buildQueryRequest method --- src/bigquery.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bigquery.ts b/src/bigquery.ts index 21d891e8..abcbff43 100644 --- a/src/bigquery.ts +++ b/src/bigquery.ts @@ -2118,7 +2118,7 @@ export class BigQuery extends Service { typeof optionsOrCallback === 'function' ? optionsOrCallback : cb; this.trace('[query]', query, options); - const queryReq = this.buildQueryRequest(query, options); + const queryReq = this.buildQueryRequest_(query, options); if (!queryReq) { this.createQueryJob(query, (err, job, resp) => { if (err) { @@ -2181,11 +2181,11 @@ export class BigQuery extends Service { * @param options QueryOptions * @returns bigquery.IQueryRequest | undefined */ - private buildQueryRequest( + private buildQueryRequest_( query: string | Query, options: QueryOptions ): bigquery.IQueryRequest | undefined { - this.trace('[probeFastPath_]', query, options); + this.trace('[buildQueryRequest]', query, options); if (process.env.FAST_QUERY_PATH === 'DISABLED') { return undefined; } From 1a3be391a2e5f2527b272a7bb848dbe6edb2ef97 Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Wed, 28 Feb 2024 14:31:44 -0400 Subject: [PATCH 4/9] feat: re-generate types --- src/types.d.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/types.d.ts b/src/types.d.ts index ae22e688..ffedd554 100644 --- a/src/types.d.ts +++ b/src/types.d.ts @@ -1,4 +1,4 @@ -// Copyright 2023 Google LLC +// Copyright 2024 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -931,7 +931,7 @@ declare namespace bigquery { */ etag?: string; /** - * Optional. Information about the external metadata storage where the dataset is defined. Filled out when the dataset type is EXTERNAL. + * Optional. Reference to a read-only external dataset defined in data catalogs outside of BigQuery. Filled out when the dataset type is EXTERNAL. */ externalDatasetReference?: IExternalDatasetReference; /** @@ -970,6 +970,10 @@ declare namespace bigquery { * Optional. Defines the time travel window in hours. The value can be from 48 to 168 hours (2 to 7 days). The default value is 168 hours if this is not set. */ maxTimeTravelHours?: string; + /** + * Output only. Reserved for future use. + */ + satisfiesPzi?: boolean; /** * Output only. Reserved for future use. */ @@ -1927,8 +1931,8 @@ declare namespace bigquery { | 'ESTIMATED_PERFORMANCE_GAIN_TOO_LOW' | 'NOT_SUPPORTED_IN_STANDARD_EDITION' | 'INDEX_SUPPRESSED_BY_FUNCTION_OPTION' - | 'INTERNAL_ERROR' | 'QUERY_CACHE_HIT' + | 'INTERNAL_ERROR' | 'OTHER_REASON'; /** * Specifies the name of the unused search index, if available. @@ -2212,6 +2216,10 @@ declare namespace bigquery { * Optional. Connection properties which can modify the load job behavior. Currently, only the 'session_id' connection property is supported, and is used to resolve _SESSION appearing as the dataset id. */ connectionProperties?: Array; + /** + * Optional. [Experimental] Configures the load job to only copy files to the destination BigLake managed table with an external storage_uri, without reading file content and writing them to new files. Copying files only is supported when: * source_uris are in the same external storage system as the destination table but they do not overlap with storage_uri of the destination table. * source_format is the same file format as the destination table. * destination_table is an existing BigLake managed table. Its schema does not have default value expression. It schema does not have type parameters other than precision and scale. * No options other than the above are specified. + */ + copyFilesOnly?: boolean; /** * Optional. Specifies whether the job is allowed to create new tables. The following values are supported: * CREATE_IF_NEEDED: If the table does not exist, BigQuery creates the table. * CREATE_NEVER: The table must already exist. If it does not, a 'notFound' error is returned in the job result. The default value is CREATE_IF_NEEDED. Creation, truncation and append actions occur as one atomic update upon job completion. */ @@ -2880,7 +2888,7 @@ declare namespace bigquery { */ undeclaredQueryParameters?: Array; /** - * Output only. Search query specific statistics. + * Output only. Vector Search query specific statistics. */ vectorSearchStatistics?: IVectorSearchStatistics; }; From 0717724a81fc89c58fd273e1160d19b5d28f4ad0 Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Wed, 28 Feb 2024 17:00:27 -0400 Subject: [PATCH 5/9] feat: support stateless queries --- src/bigquery.ts | 21 ++++++++++----------- src/job.ts | 11 +++++++---- test/bigquery.ts | 16 ++++++++++++++++ 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/bigquery.ts b/src/bigquery.ts index 862677d8..2bccb2c1 100644 --- a/src/bigquery.ts +++ b/src/bigquery.ts @@ -1436,6 +1436,7 @@ export class BigQuery extends Service { callback?: JobCallback ): void | Promise { const options = typeof opts === 'object' ? opts : {query: opts}; + this.trace('[createQueryJob]', options, callback); if ((!options || !options.query) && !options.pageToken) { throw new Error('A SQL query string is required.'); } @@ -2143,7 +2144,7 @@ export class BigQuery extends Service { return; } - let nextQuery = extend({job}, options); + options = extend({job}, queryOpts, options); if (res && res.jobComplete) { let rows: any = []; if (res.schema && res.rows) { @@ -2152,20 +2153,16 @@ export class BigQuery extends Service { parseJSON: options.parseJSON, }); } + options.cachedRows = rows; this.trace('[runJobsQuery] job complete'); if (res.pageToken) { this.trace('[runJobsQuery] has more pages'); - nextQuery = extend({job}, options, { - pageToken: res.pageToken, - cachedRows: rows, - }); - job!.getQueryResults(nextQuery, callback as QueryRowsCallback); - return; + options.pageToken = res.pageToken; } else { this.trace('[runJobsQuery] no more pages'); - (callback as SimpleQueryRowsCallback)(err, rows, res); - return; } + job!.getQueryResults(options, callback as QueryRowsCallback); + return; } this.trace('[runJobsQuery] job not complete'); job!.getQueryResults(options, callback as QueryRowsCallback); @@ -2176,8 +2173,8 @@ export class BigQuery extends Service { * Check if the given Query can run using the `jobs.query` endpoint. * Returns a bigquery.IQueryRequest that can be used to call `jobs.query`. * Return undefined if is not possible to convert to a bigquery.IQueryRequest. - * - * @param query string | Query + * + * @param query string | Query * @param options QueryOptions * @returns bigquery.IQueryRequest | undefined */ @@ -2245,6 +2242,8 @@ export class BigQuery extends Service { job = this.job(jobRef.jobId!, { location: jobRef.location, }); + } else { + job = this.job(res.queryId!); // stateless query } callback!(null, job, res); } diff --git a/src/job.ts b/src/job.ts index ccd9c941..2f9503dc 100644 --- a/src/job.ts +++ b/src/job.ts @@ -561,10 +561,13 @@ class Job extends Operation { typeof qs.timeoutMs === 'number' ? qs.timeoutMs : false; if (options.cachedRows) { - const nextQuery = Object.assign({}, options, { - pageToken: options.pageToken, - }); - delete nextQuery.cachedRows; + let nextQuery: QueryResultsOptions | null = null; + if (options.pageToken) { + nextQuery = Object.assign({}, options, { + pageToken: options.pageToken, + }); + delete nextQuery.cachedRows; + } callback!(null, options.cachedRows, nextQuery); return; } diff --git a/test/bigquery.ts b/test/bigquery.ts index 914980ac..fb623407 100644 --- a/test/bigquery.ts +++ b/test/bigquery.ts @@ -2812,6 +2812,10 @@ describe('BigQuery', () => { callback(error, null, FAKE_RESPONSE); }; + bq.buildQueryRequest_ = (query: {}, options: {}) => { + return undefined; + }; + bq.query(QUERY_STRING, (err: Error, rows: {}, resp: {}) => { assert.strictEqual(err, error); assert.strictEqual(rows, null); @@ -2850,6 +2854,10 @@ describe('BigQuery', () => { callback(null, fakeJob, FAKE_RESPONSE); }; + bq.buildQueryRequest_ = (query: {}, options: {}) => { + return undefined; + }; + bq.query(QUERY_STRING, (err: Error, rows: {}, resp: {}) => { assert.ifError(err); assert.strictEqual(rows, FAKE_ROWS); @@ -2901,6 +2909,10 @@ describe('BigQuery', () => { callback(null, fakeJob, FAKE_RESPONSE); }; + bq.buildQueryRequest_ = (query: {}, opts: {}) => { + return undefined; + }; + bq.query(QUERY_STRING, assert.ifError); }); @@ -2918,6 +2930,10 @@ describe('BigQuery', () => { callback(null, fakeJob, FAKE_RESPONSE); }; + bq.buildQueryRequest_ = (query: {}, opts: {}) => { + return undefined; + }; + bq.query(QUERY_STRING, fakeOptions, assert.ifError); }); }); From 970e08f190594669011d49880939fe24b1856a0a Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Mon, 11 Mar 2024 10:46:06 -0400 Subject: [PATCH 6/9] feat: fully build QueryRequest object for jobs.query --- src/bigquery.ts | 287 ++++++++++++++++++++++++++++------------------- src/job.ts | 6 +- test/bigquery.ts | 221 ++++++++++++++++++++++++++++++++++-- 3 files changed, 386 insertions(+), 128 deletions(-) diff --git a/src/bigquery.ts b/src/bigquery.ts index 2bccb2c1..acff69f2 100644 --- a/src/bigquery.ts +++ b/src/bigquery.ts @@ -493,7 +493,7 @@ export class BigQuery extends Service { } // eslint-disable-next-line @typescript-eslint/no-explicit-any - private trace(msg: string, ...otherArgs: any[]) { + private trace_(msg: string, ...otherArgs: any[]) { logger('[bigquery]', msg, ...otherArgs); } @@ -1436,20 +1436,19 @@ export class BigQuery extends Service { callback?: JobCallback ): void | Promise { const options = typeof opts === 'object' ? opts : {query: opts}; - this.trace('[createQueryJob]', options, callback); + this.trace_('[createQueryJob]', options, callback); if ((!options || !options.query) && !options.pageToken) { throw new Error('A SQL query string is required.'); } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const query: any = extend( + const query: Query = extend( true, { useLegacySql: false, }, options ); - this.trace('[createQueryJob]', query); + this.trace_('[createQueryJob]', query); if (options.destination) { if (!(options.destination instanceof Table)) { @@ -1465,78 +1464,21 @@ export class BigQuery extends Service { delete query.destination; } - if (query.params) { - query.parameterMode = is.array(query.params) ? 'positional' : 'named'; - - if (query.parameterMode === 'named') { - query.queryParameters = []; - - // tslint:disable-next-line forin - for (const namedParameter in query.params) { - const value = query.params[namedParameter]; - let queryParameter; - - if (query.types) { - if (!is.object(query.types)) { - throw new Error( - 'Provided types must match the value type passed to `params`' - ); - } - - if (query.types[namedParameter]) { - queryParameter = BigQuery.valueToQueryParameter_( - value, - query.types[namedParameter] - ); - } else { - queryParameter = BigQuery.valueToQueryParameter_(value); - } - } else { - queryParameter = BigQuery.valueToQueryParameter_(value); - } - - queryParameter.name = namedParameter; - query.queryParameters.push(queryParameter); - } - } else { - query.queryParameters = []; - - if (query.types) { - if (!is.array(query.types)) { - throw new Error( - 'Provided types must match the value type passed to `params`' - ); - } - - if (query.params.length !== query.types.length) { - throw new Error('Incorrect number of parameter types provided.'); - } - query.params.forEach((value: {}, i: number) => { - const queryParameter = BigQuery.valueToQueryParameter_( - value, - query.types[i] - ); - query.queryParameters.push(queryParameter); - }); - } else { - query.params.forEach((value: {}) => { - const queryParameter = BigQuery.valueToQueryParameter_(value); - query.queryParameters.push(queryParameter); - }); - } - } - delete query.params; - } + const {parameterMode, params} = this.buildQueryParams_( + query.params, + query.types + ); + query.parameterMode = parameterMode; + query.queryParameters = params; + delete query.params; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const reqOpts: any = { - configuration: { - query, - }, + const reqOpts: JobOptions = {}; + reqOpts.configuration = { + query, }; if (typeof query.jobTimeoutMs === 'number') { - reqOpts.configuration.jobTimeoutMs = query.jobTimeoutMs; + reqOpts.configuration.jobTimeoutMs = query.jobTimeoutMs.toString(); delete query.jobTimeoutMs; } @@ -1568,6 +1510,85 @@ export class BigQuery extends Service { this.createJob(reqOpts, callback!); } + private buildQueryParams_( + params: Query['params'], + types: Query['types'] + ): { + parameterMode: 'positional' | 'named' | undefined; + params: bigquery.IQueryParameter[] | undefined; + } { + if (!params) { + return { + parameterMode: undefined, + params: undefined, + }; + } + const parameterMode = is.array(params) ? 'positional' : 'named'; + const queryParameters: bigquery.IQueryParameter[] = []; + if (parameterMode === 'named') { + const namedParams = params as {[param: string]: any}; + for (const namedParameter in namedParams) { + const value = namedParams[namedParameter]; + let queryParameter; + + if (types) { + if (!is.object(types)) { + throw new Error( + 'Provided types must match the value type passed to `params`' + ); + } + + const namedTypes = types as QueryParamTypeStruct; + + if (namedTypes[namedParameter]) { + queryParameter = BigQuery.valueToQueryParameter_( + value, + namedTypes[namedParameter] + ); + } else { + queryParameter = BigQuery.valueToQueryParameter_(value); + } + } else { + queryParameter = BigQuery.valueToQueryParameter_(value); + } + + queryParameter.name = namedParameter; + queryParameters.push(queryParameter); + } + } else { + if (types) { + if (!is.array(types)) { + throw new Error( + 'Provided types must match the value type passed to `params`' + ); + } + + const positionalTypes = types as QueryParamTypeStruct[]; + + if (params.length !== types.length) { + throw new Error('Incorrect number of parameter types provided.'); + } + params.forEach((value: {}, i: number) => { + const queryParameter = BigQuery.valueToQueryParameter_( + value, + positionalTypes[i] + ); + queryParameters.push(queryParameter); + }); + } else { + params.forEach((value: {}) => { + const queryParameter = BigQuery.valueToQueryParameter_(value); + queryParameters.push(queryParameter); + }); + } + } + + return { + parameterMode, + params: queryParameters, + }; + } + /** * Creates a job. Typically when creating a job you'll have a very specific * task in mind. For this we recommend one of the following methods: @@ -2118,8 +2139,9 @@ export class BigQuery extends Service { const callback = typeof optionsOrCallback === 'function' ? optionsOrCallback : cb; - this.trace('[query]', query, options); + this.trace_('[query]', query, options); const queryReq = this.buildQueryRequest_(query, options); + this.trace_('[query] queryReq', queryReq); if (!queryReq) { this.createQueryJob(query, (err, job, resp) => { if (err) { @@ -2138,7 +2160,8 @@ export class BigQuery extends Service { return; } - this.runJobsQuery(queryReq, options, (err, job, res) => { + this.runJobsQuery(queryReq, (err, job, res) => { + this.trace_('[runJobsQuery callback]: ', query, err, job, res); if (err) { (callback as SimpleQueryRowsCallback)(err, null, res); return; @@ -2154,17 +2177,17 @@ export class BigQuery extends Service { }); } options.cachedRows = rows; - this.trace('[runJobsQuery] job complete'); + this.trace_('[runJobsQuery] job complete'); if (res.pageToken) { - this.trace('[runJobsQuery] has more pages'); + this.trace_('[runJobsQuery] has more pages'); options.pageToken = res.pageToken; } else { - this.trace('[runJobsQuery] no more pages'); + this.trace_('[runJobsQuery] no more pages'); } job!.getQueryResults(options, callback as QueryRowsCallback); return; } - this.trace('[runJobsQuery] job not complete'); + this.trace_('[runJobsQuery] job not complete'); job!.getQueryResults(options, callback as QueryRowsCallback); }); } @@ -2182,48 +2205,84 @@ export class BigQuery extends Service { query: string | Query, options: QueryOptions ): bigquery.IQueryRequest | undefined { - this.trace('[buildQueryRequest]', query, options); if (process.env.FAST_QUERY_PATH === 'DISABLED') { return undefined; } - if (typeof query === 'string') { - if (!options.job) { - const req: bigquery.IQueryRequest = { - ...options, - useQueryCache: false, - jobCreationMode: 'JOB_CREATION_OPTIONAL', - useLegacySql: false, - requestId: uuid.v4(), - query: query, - }; - return req; - } + const queryObj: Query = + typeof query === 'string' + ? { + query: query, + } + : query; + this.trace_('[buildQueryRequest]', query, options, queryObj); + // This is a denylist of settings which prevent us from composing an equivalent + // bq.QueryRequest due to differences between configuration parameters accepted + // by jobs.insert vs jobs.query. + if ( + !!queryObj.destination || + !!queryObj.tableDefinitions || + !!queryObj.createDisposition || + !!queryObj.writeDisposition || + (!!queryObj.priority && queryObj.priority !== 'INTERACTIVE') || + queryObj.useLegacySql || + !!queryObj.maximumBillingTier || + !!queryObj.timePartitioning || + !!queryObj.rangePartitioning || + !!queryObj.clustering || + !!queryObj.destinationEncryptionConfiguration || + !!queryObj.schemaUpdateOptions || + !!queryObj.jobTimeoutMs || + // User has defined the jobID generation behavior + !!queryObj.jobId + ) { return undefined; } - // TODO: non string query and convert to QueryRequest - return undefined; + + if (queryObj.dryRun) { + return undefined; + } + + if (options.job) { + return undefined; + } + const req: bigquery.IQueryRequest = { + useQueryCache: queryObj.useQueryCache, + labels: queryObj.labels, + defaultDataset: queryObj.defaultDataset, + createSession: queryObj.createSession, + maximumBytesBilled: queryObj.maximumBytesBilled, + timeoutMs: options.timeoutMs, + location: queryObj.location || options.location, + formatOptions: { + useInt64Timestamp: true, + }, + maxResults: queryObj.maxResults || options.maxResults, + query: queryObj.query, + useLegacySql: false, + requestId: uuid.v4(), + jobCreationMode: 'JOB_CREATION_OPTIONAL', + }; + if (req.maxResults) { + req.jobCreationMode = 'JOB_CREATION_REQUIRED'; + } + const {parameterMode, params} = this.buildQueryParams_( + queryObj.params, + queryObj.types + ); + if (params) { + req.queryParameters = params; + } + if (parameterMode) { + req.parameterMode = parameterMode; + } + return req; } private runJobsQuery( req: bigquery.IQueryRequest, - options?: QueryResultsOptions - ): Promise; - private runJobsQuery( - req: bigquery.IQueryRequest, - options: QueryResultsOptions, - callback: JobsQueryCallback - ): void; - private runJobsQuery( - req: bigquery.IQueryRequest, - optionsOrCallback?: QueryResultsOptions | JobsQueryCallback, - cb?: JobsQueryCallback + callback?: JobsQueryCallback ): void | Promise { - const options = - typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; - const callback = - typeof optionsOrCallback === 'function' ? optionsOrCallback : cb; - - this.trace('[runJobsQuery]', req, options, callback); + this.trace_('[runJobsQuery]', req, callback); this.request( { method: 'POST', @@ -2231,19 +2290,19 @@ export class BigQuery extends Service { json: req, }, async (err, res: bigquery.IQueryResponse) => { + this.trace_('jobs.query res:', res, err); if (err) { callback!(err, null, res); return; } - this.trace('jobs.query res:', res); let job: Job | null = null; if (res.jobReference) { const jobRef = res.jobReference!; job = this.job(jobRef.jobId!, { location: jobRef.location, }); - } else { - job = this.job(res.queryId!); // stateless query + } else if (res.queryId) { + job = this.job(res.queryId); // stateless query } callback!(null, job, res); } diff --git a/src/job.ts b/src/job.ts index 2f9503dc..d1816302 100644 --- a/src/job.ts +++ b/src/job.ts @@ -382,7 +382,7 @@ class Job extends Operation { } // eslint-disable-next-line @typescript-eslint/no-explicit-any - private trace(msg: string, ...otherArgs: any[]) { + private trace_(msg: string, ...otherArgs: any[]) { logger(`[job][${this.id}]`, msg, ...otherArgs); } @@ -543,7 +543,7 @@ class Job extends Operation { }, options ); - this.trace( + this.trace_( '[getQueryResults]', this.id, options.pageToken, @@ -607,7 +607,7 @@ class Job extends Operation { return; } } else if (resp.pageToken) { - this.trace('[getQueryResults] has more pages', resp.pageToken); + this.trace_('[getQueryResults] has more pages', resp.pageToken); // More results exist. nextQuery = Object.assign({}, options, { pageToken: resp.pageToken, diff --git a/test/bigquery.ts b/test/bigquery.ts index fb623407..d3519d3d 100644 --- a/test/bigquery.ts +++ b/test/bigquery.ts @@ -40,6 +40,8 @@ import { Table, JobOptions, TableField, + Query, + QueryResultsOptions, } from '../src'; import {SinonStub} from 'sinon'; import {PreciseDate} from '@google-cloud/precise-date'; @@ -2117,10 +2119,10 @@ describe('BigQuery', () => { it('should set the correct query parameters', done => { const queryParameter = {}; - BigQuery.valueToQueryParameter_ = (value: {}) => { + sandbox.replace(BigQuery, 'valueToQueryParameter_', (value: {}) => { assert.strictEqual(value, NAMED_PARAMS.key); return queryParameter; - }; + }); bq.createJob = (reqOpts: JobOptions) => { const query = reqOpts.configuration!.query!; @@ -2141,14 +2143,14 @@ describe('BigQuery', () => { it('should allow for optional parameter types', () => { const queryParameter = {}; - BigQuery.valueToQueryParameter_ = ( + sandbox.replace(BigQuery, 'valueToQueryParameter_', ( value: {}, providedType: string ) => { assert.strictEqual(value, NAMED_PARAMS.key); assert.strictEqual(providedType, NAMED_TYPES.key); return queryParameter; - }; + }); bq.createJob = (reqOpts: JobOptions) => { // eslint-disable-next-line @typescript-eslint/no-explicit-any assert.strictEqual((reqOpts as any).params, undefined); @@ -2167,10 +2169,10 @@ describe('BigQuery', () => { it('should allow for providing only some parameter types', () => { const queryParameter = {}; - BigQuery.valueToQueryParameter_ = (value: {}) => { + sandbox.replace(BigQuery, 'valueToQueryParameter_', (value: {}) => { assert.strictEqual(value, NAMED_PARAMS.key); return queryParameter; - }; + }); bq.createJob = (reqOpts: JobOptions) => { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -2202,9 +2204,9 @@ describe('BigQuery', () => { it('should set the correct parameter mode', done => { const queryParameter = {}; - BigQuery.valueToQueryParameter_ = () => { + sandbox.replace(BigQuery, 'valueToQueryParameter_', (value: {}) => { return queryParameter; - }; + }); bq.createJob = (reqOpts: JobOptions) => { const query = reqOpts.configuration!.query!; @@ -2224,10 +2226,10 @@ describe('BigQuery', () => { it('should set the correct query parameters', done => { const queryParameter = {}; - BigQuery.valueToQueryParameter_ = (value: {}) => { + sandbox.replace(BigQuery, 'valueToQueryParameter_', (value: {}) => { assert.strictEqual(value, POSITIONAL_PARAMS[0]); return queryParameter; - }; + }); bq.createJob = (reqOpts: JobOptions) => { const query = reqOpts.configuration!.query!; @@ -2417,7 +2419,7 @@ describe('BigQuery', () => { ); assert.strictEqual( reqOpts.configuration!.jobTimeoutMs, - options.jobTimeoutMs + `${options.jobTimeoutMs}` ); done(); }; @@ -2824,6 +2826,21 @@ describe('BigQuery', () => { }); }); + it('should return any errors from jobs.query', done => { + const error = new Error('err'); + + bq.runJobsQuery = (query: {}, callback: Function) => { + callback(error, null, FAKE_RESPONSE); + }; + + bq.query(QUERY_STRING, (err: Error, rows: {}, resp: {}) => { + assert.strictEqual(err, error); + assert.strictEqual(rows, null); + assert.strictEqual(resp, FAKE_RESPONSE); + done(); + }); + }); + it('should exit early if dryRun is set', done => { const options = { query: QUERY_STRING, @@ -2835,6 +2852,10 @@ describe('BigQuery', () => { callback(null, null, FAKE_RESPONSE); }; + bq.buildQueryRequest_ = (query: {}, options: {}) => { + return undefined; + }; + bq.query(options, (err: Error, rows: {}, resp: {}) => { assert.ifError(err); assert.deepStrictEqual(rows, []); @@ -2866,6 +2887,43 @@ describe('BigQuery', () => { }); }); + it('should call job#getQueryResults with cached rows from jobs.query', done => { + const fakeJob = { + getQueryResults: (options: QueryResultsOptions, callback: Function) => { + callback(null, options.cachedRows, FAKE_RESPONSE); + }, + }; + + 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}]}, + ], + }); + }; + + bq.query(QUERY_STRING, (err: Error, rows: {}, resp: {}) => { + assert.ifError(err); + assert.deepStrictEqual(rows, [{ + value: 1, + },{ + value: 2 + },{ + value: 3 + }]); + assert.strictEqual(resp, FAKE_RESPONSE); + done(); + }); + }); + it('should call job#getQueryResults with query options', done => { let queryResultsOpts = {}; const fakeJob = { @@ -2879,6 +2937,10 @@ describe('BigQuery', () => { callback(null, fakeJob, FAKE_RESPONSE); }; + bq.buildQueryRequest_ = (query: {}, options: {}) => { + return undefined; + }; + const query = { query: QUERY_STRING, wrapIntegers: true, @@ -2938,6 +3000,143 @@ describe('BigQuery', () => { }); }); + describe('buildQueryRequest_', () => { + const DATASET_ID = 'dataset-id'; + const TABLE_ID = 'table-id'; + const QUERY_STRING = 'SELECT * FROM [dataset.table]'; + + it('should create a QueryRequest from a Query interface', () => { + const q: Query = { + query: QUERY_STRING, + maxResults: 10, + defaultDataset: { + projectId: PROJECT_ID, + datasetId: DATASET_ID, + }, + priority: 'INTERACTIVE', + params: { + key: 'value', + }, + maximumBytesBilled: '1024', + labels: { + key: 'value', + }, + }; + const req = bq.buildQueryRequest_(q, {}); + for (let key in req) { + if (req[key] === undefined) { + delete req[key]; + } + } + const expectedReq = { + query: QUERY_STRING, + useLegacySql: false, + requestId: req.requestId, + maxResults: 10, + defaultDataset: { + projectId: PROJECT_ID, + datasetId: DATASET_ID, + }, + parameterMode: 'named', + queryParameters: [ + { + name: 'key', + parameterType: { + type: 'STRING', + }, + parameterValue: { + value: 'value', + }, + }, + ], + maximumBytesBilled: '1024', + labels: { + key: 'value', + }, + jobCreationMode: 'JOB_CREATION_REQUIRED', // due to maxResults set + formatOptions: { + useInt64Timestamp: true, + }, + }; + assert.deepStrictEqual(req, expectedReq); + }); + + it('should create a QueryRequest from a SQL string', () => { + const req = bq.buildQueryRequest_(QUERY_STRING, {}); + for (let key in req) { + if (req[key] === undefined) { + delete req[key]; + } + } + const expectedReq = { + query: QUERY_STRING, + useLegacySql: false, + requestId: req.requestId, + jobCreationMode: 'JOB_CREATION_OPTIONAL', + formatOptions: { + useInt64Timestamp: true, + }, + }; + assert.deepStrictEqual(req, expectedReq); + }); + + it('should not create a QueryRequest when config is not accepted by jobs.query', () => { + const dataset: any = { + bigQuery: bq, + id: 'dataset-id', + createTable: util.noop, + }; + const table = new FakeTable(dataset, TABLE_ID); + const testCases: Query[] = [ + { + query: QUERY_STRING, + dryRun: true, + }, + { + query: QUERY_STRING, + destination: table, + }, + { + query: QUERY_STRING, + clustering: { + fields: ['date'], + }, + }, + { + query: QUERY_STRING, + clustering: {}, + }, + { + query: QUERY_STRING, + timePartitioning: {}, + }, + { + query: QUERY_STRING, + rangePartitioning: {}, + }, + { + query: QUERY_STRING, + jobId: 'fixed-job-id' + }, + { + query: QUERY_STRING, + createDisposition: 'CREATED_IF_NEEDED', + writeDisposition: 'WRITE_APPEND', + }, + { + query: QUERY_STRING, + schemaUpdateOptions: ['update'] + } + ]; + + for (let index in testCases) { + const testCase = testCases[index]; + const req = bq.buildQueryRequest_(testCase, {}); + assert.equal(req, undefined); + } + }); + }); + describe('queryAsStream_', () => { let queryStub: SinonStub; const defaultOpts = { From bce8c4866b66b38c257843eef595f1f90184ea71 Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Tue, 12 Mar 2024 10:26:58 -0400 Subject: [PATCH 7/9] fix: lint issues --- test/bigquery.ts | 57 ++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/test/bigquery.ts b/test/bigquery.ts index d3519d3d..2707cc41 100644 --- a/test/bigquery.ts +++ b/test/bigquery.ts @@ -2143,14 +2143,15 @@ describe('BigQuery', () => { it('should allow for optional parameter types', () => { const queryParameter = {}; - sandbox.replace(BigQuery, 'valueToQueryParameter_', ( - value: {}, - providedType: string - ) => { - assert.strictEqual(value, NAMED_PARAMS.key); - assert.strictEqual(providedType, NAMED_TYPES.key); - return queryParameter; - }); + sandbox.replace( + BigQuery, + 'valueToQueryParameter_', + (value: {}, providedType: string) => { + assert.strictEqual(value, NAMED_PARAMS.key); + assert.strictEqual(providedType, NAMED_TYPES.key); + return queryParameter; + } + ); bq.createJob = (reqOpts: JobOptions) => { // eslint-disable-next-line @typescript-eslint/no-explicit-any assert.strictEqual((reqOpts as any).params, undefined); @@ -2898,27 +2899,25 @@ describe('BigQuery', () => { callback(null, fakeJob, { jobComplete: true, schema: { - fields: [ - {name: 'value', type: 'INT64'} - ] + fields: [{name: 'value', type: 'INT64'}], }, - rows: [ - {f: [{v: 1}]}, - {f: [{v: 2}]}, - {f: [{v: 3}]}, - ], + rows: [{f: [{v: 1}]}, {f: [{v: 2}]}, {f: [{v: 3}]}], }); }; bq.query(QUERY_STRING, (err: Error, rows: {}, resp: {}) => { assert.ifError(err); - assert.deepStrictEqual(rows, [{ - value: 1, - },{ - value: 2 - },{ - value: 3 - }]); + assert.deepStrictEqual(rows, [ + { + value: 1, + }, + { + value: 2, + }, + { + value: 3, + }, + ]); assert.strictEqual(resp, FAKE_RESPONSE); done(); }); @@ -3023,7 +3022,7 @@ describe('BigQuery', () => { }, }; const req = bq.buildQueryRequest_(q, {}); - for (let key in req) { + for (const key in req) { if (req[key] === undefined) { delete req[key]; } @@ -3063,7 +3062,7 @@ describe('BigQuery', () => { it('should create a QueryRequest from a SQL string', () => { const req = bq.buildQueryRequest_(QUERY_STRING, {}); - for (let key in req) { + for (const key in req) { if (req[key] === undefined) { delete req[key]; } @@ -3116,7 +3115,7 @@ describe('BigQuery', () => { }, { query: QUERY_STRING, - jobId: 'fixed-job-id' + jobId: 'fixed-job-id', }, { query: QUERY_STRING, @@ -3125,11 +3124,11 @@ describe('BigQuery', () => { }, { query: QUERY_STRING, - schemaUpdateOptions: ['update'] - } + schemaUpdateOptions: ['update'], + }, ]; - for (let index in testCases) { + for (const index in testCases) { const testCase = testCases[index]; const req = bq.buildQueryRequest_(testCase, {}); assert.equal(req, undefined); From 9a2cce5299dce6b03ccd9bd3a2da92bd6fcfd33a Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Tue, 19 Mar 2024 16:28:57 -0400 Subject: [PATCH 8/9] feat: only use jobCreationMode with preview feature enable --- src/bigquery.ts | 21 +++++++++++++++++++++ test/bigquery.ts | 1 + 2 files changed, 22 insertions(+) diff --git a/src/bigquery.ts b/src/bigquery.ts index 76bd1406..e9be29bd 100644 --- a/src/bigquery.ts +++ b/src/bigquery.ts @@ -286,6 +286,12 @@ export const PROTOCOL_REGEX = /^(\w*):\/\//; * We will create a table with the correct schema, import the public CSV file * into that table, and query it for data. * + * This client supports enabling query-related preview features via environmental + * variables. By setting the environment variable QUERY_PREVIEW_ENABLED to the string + * "TRUE", the client will enable preview features, though behavior may still be + * controlled via the bigquery service as well. Currently, the feature(s) in scope + * include: stateless queries (query execution without corresponding job metadata). + * * @class * * See {@link https://cloud.google.com/bigquery/what-is-bigquery| What is BigQuery?} @@ -322,6 +328,7 @@ export const PROTOCOL_REGEX = /^(\w*):\/\//; export class BigQuery extends Service { location?: string; private _universeDomain: string; + private _enableQueryPreview: boolean; createQueryStream(options?: Query | string): ResourceStream { // placeholder body, overwritten in constructor @@ -379,6 +386,14 @@ export class BigQuery extends Service { super(config, options); + const QUERY_PREVIEW_ENABLED = process.env.QUERY_PREVIEW_ENABLED; + this._enableQueryPreview = false; + if (typeof QUERY_PREVIEW_ENABLED === 'string') { + if (QUERY_PREVIEW_ENABLED.toUpperCase() === 'TRUE') { + this._enableQueryPreview = true; + } + } + this._universeDomain = universeDomain; this.location = options.location; /** @@ -2195,6 +2210,9 @@ export class BigQuery extends Service { job!.getQueryResults(options, callback as QueryRowsCallback); return; } + if (options.timeoutMs) { + delete options.timeoutMs; + } this.trace_('[runJobsQuery] job not complete'); job!.getQueryResults(options, callback as QueryRowsCallback); }); @@ -2273,6 +2291,9 @@ export class BigQuery extends Service { if (req.maxResults) { req.jobCreationMode = 'JOB_CREATION_REQUIRED'; } + if (!this._enableQueryPreview) { + delete req.jobCreationMode; + } const {parameterMode, params} = this.buildQueryParams_( queryObj.params, queryObj.types diff --git a/test/bigquery.ts b/test/bigquery.ts index caface07..eea641f4 100644 --- a/test/bigquery.ts +++ b/test/bigquery.ts @@ -189,6 +189,7 @@ describe('BigQuery', () => { Object.assign(fakeUtil, originalFakeUtil); BigQuery = Object.assign(BigQuery, BigQueryCached); bq = new BigQuery({projectId: PROJECT_ID}); + bq._enableQueryPreview = true; }); after(() => { From 7531a12524fa173159682a1b3cb63a432b7e1f95 Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Tue, 26 Mar 2024 11:42:33 -0400 Subject: [PATCH 9/9] fix: address code review comments --- src/bigquery.ts | 18 +++++++----------- src/job.ts | 14 +++++++++----- src/logger.ts | 2 +- test/bigquery.ts | 4 ++-- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/bigquery.ts b/src/bigquery.ts index e9be29bd..010a7071 100644 --- a/src/bigquery.ts +++ b/src/bigquery.ts @@ -198,6 +198,7 @@ export interface ProvidedTypeStruct { } export type QueryParameter = bigquery.IQueryParameter; +export type ParameterMode = bigquery.IJobConfigurationQuery['parameterMode']; export interface BigQueryOptions extends GoogleAuthOptions { /** @@ -1537,7 +1538,7 @@ export class BigQuery extends Service { params: Query['params'], types: Query['types'] ): { - parameterMode: 'positional' | 'named' | undefined; + parameterMode: ParameterMode; params: bigquery.IQueryParameter[] | undefined; } { if (!params) { @@ -1550,7 +1551,7 @@ export class BigQuery extends Service { const queryParameters: bigquery.IQueryParameter[] = []; if (parameterMode === 'named') { const namedParams = params as {[param: string]: any}; - for (const namedParameter in namedParams) { + for (const namedParameter of Object.getOwnPropertyNames(namedParams)) { const value = namedParams[namedParameter]; let queryParameter; @@ -2195,12 +2196,12 @@ export class BigQuery extends Service { let rows: any = []; if (res.schema && res.rows) { rows = BigQuery.mergeSchemaWithRows_(res.schema, res.rows, { - wrapIntegers: options.wrapIntegers!, // TODO: fix default value + wrapIntegers: options.wrapIntegers || false, parseJSON: options.parseJSON, }); } - options.cachedRows = rows; this.trace_('[runJobsQuery] job complete'); + options._cachedRows = rows; if (res.pageToken) { this.trace_('[runJobsQuery] has more pages'); options.pageToken = res.pageToken; @@ -2210,9 +2211,7 @@ export class BigQuery extends Service { job!.getQueryResults(options, callback as QueryRowsCallback); return; } - if (options.timeoutMs) { - delete options.timeoutMs; - } + delete options.timeoutMs; this.trace_('[runJobsQuery] job not complete'); job!.getQueryResults(options, callback as QueryRowsCallback); }); @@ -2288,9 +2287,6 @@ export class BigQuery extends Service { requestId: uuid.v4(), jobCreationMode: 'JOB_CREATION_OPTIONAL', }; - if (req.maxResults) { - req.jobCreationMode = 'JOB_CREATION_REQUIRED'; - } if (!this._enableQueryPreview) { delete req.jobCreationMode; } @@ -2326,7 +2322,7 @@ export class BigQuery extends Service { } let job: Job | null = null; if (res.jobReference) { - const jobRef = res.jobReference!; + const jobRef = res.jobReference; job = this.job(jobRef.jobId!, { location: jobRef.location, }); diff --git a/src/job.ts b/src/job.ts index d1816302..a950d1ed 100644 --- a/src/job.ts +++ b/src/job.ts @@ -51,8 +51,12 @@ export type QueryResultsOptions = { job?: Job; wrapIntegers?: boolean | IntegerTypeCastOptions; parseJSON?: boolean; - cachedRows?: any[]; -} & PagedRequest; +} & PagedRequest & { + /** + * internal properties + */ + _cachedRows?: any[]; + }; /** * @callback QueryResultsCallback @@ -560,15 +564,15 @@ class Job extends Operation { const timeoutOverride = typeof qs.timeoutMs === 'number' ? qs.timeoutMs : false; - if (options.cachedRows) { + if (options._cachedRows) { let nextQuery: QueryResultsOptions | null = null; if (options.pageToken) { nextQuery = Object.assign({}, options, { pageToken: options.pageToken, }); - delete nextQuery.cachedRows; + delete nextQuery._cachedRows; } - callback!(null, options.cachedRows, nextQuery); + callback!(null, options._cachedRows, nextQuery); return; } diff --git a/src/logger.ts b/src/logger.ts index c3ff69e7..d8dfc017 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -37,7 +37,7 @@ export function logger(source: string, msg: string, ...otherArgs: any[]) { } /** - * Sets or disables the log function for all active Firestore instances. + * Sets or disables the log function for all active BigQuery instances. * * @param logger A log function that takes a message (such as `console.log`) or * `null` to turn off logging. diff --git a/test/bigquery.ts b/test/bigquery.ts index eea641f4..b1022acd 100644 --- a/test/bigquery.ts +++ b/test/bigquery.ts @@ -2880,7 +2880,7 @@ describe('BigQuery', () => { it('should call job#getQueryResults with cached rows from jobs.query', done => { const fakeJob = { getQueryResults: (options: QueryResultsOptions, callback: Function) => { - callback(null, options.cachedRows, FAKE_RESPONSE); + callback(null, options._cachedRows, FAKE_RESPONSE); }, }; @@ -3041,7 +3041,7 @@ describe('BigQuery', () => { labels: { key: 'value', }, - jobCreationMode: 'JOB_CREATION_REQUIRED', // due to maxResults set + jobCreationMode: 'JOB_CREATION_OPTIONAL', formatOptions: { useInt64Timestamp: true, },