From f31d7b205d74d4a783f0d5159dd5b62efe968fe6 Mon Sep 17 00:00:00 2001 From: alkatrivedi <58396306+alkatrivedi@users.noreply.github.com> Date: Fri, 17 May 2024 04:53:59 +0000 Subject: [PATCH] Fix: drop table statement (#2036) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: integration test fix * fix: drop table * fix(deps): update dependency google-gax to v4.3.2 (#2026) * fix(deps): update dependency google-gax to v4.3.2 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot * feat(spanner): adding `EXPECTED_FULFILLMENT_PERIOD` to the indicate instance creation times (with `FULFILLMENT_PERIOD_NORMAL` or `FULFILLMENT_PERIOD_EXTENDED` ENUM) with the extended instance creation time triggered by On-Demand Capacity Feature (#2024) * feat: add several fields to manage state of database encryption update PiperOrigin-RevId: 619289281 Source-Link: https://github.com/googleapis/googleapis/commit/3a7c33486ca758b180c6d11dd4705fa9a22e8576 Source-Link: https://github.com/googleapis/googleapis-gen/commit/6a8c733062d833d11c5245eda50f5108e0e55324 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNmE4YzczMzA2MmQ4MzNkMTFjNTI0NWVkYTUwZjUxMDhlMGU1NTMyNCJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * feat: Add SessionPoolOptions, SpannerOptions protos in executor protos PiperOrigin-RevId: 621265883 Source-Link: https://github.com/googleapis/googleapis/commit/fed9845c564d6acf8d03beee69b36666c8bd7fa4 Source-Link: https://github.com/googleapis/googleapis-gen/commit/c66a76957e2e16347bc1dd3f4c638223f065ee80 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYzY2YTc2OTU3ZTJlMTYzNDdiYzFkZDNmNGM2MzgyMjNmMDY1ZWU4MCJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * feat(spanner): adding `EXPECTED_FULFILLMENT_PERIOD` to the indicate instance creation times (with `FULFILLMENT_PERIOD_NORMAL` or `FULFILLMENT_PERIOD_EXTENDED` ENUM) with the extended instance creation time triggered by On-Demand Capacity Feature PiperOrigin-RevId: 621488048 Source-Link: https://github.com/googleapis/googleapis/commit/0aa0992a5430c211a73c9b861d65e1e8a7a91a9e Source-Link: https://github.com/googleapis/googleapis-gen/commit/b8ad4c73a5c05fed8bcfddb931326996c3441791 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYjhhZDRjNzNhNWMwNWZlZDhiY2ZkZGI5MzEzMjY5OTZjMzQ0MTc5MSJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot Co-authored-by: danieljbruce * feat: optimisticLock option for getTransaction method (#2028) * feat: add option for optimistic lock in getTransaction method * feat: add option for optimistic lock in getTransaction method * add sample for read and write transaction * add sample for read and write transaction * chore: add integration test for the read/write query samples * fix: presubmit error * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix: presubmit error * refactor: sample doc * refactor: sample doc * refator: getTransaction method * fix: lint errors * fix: lint errors * refactor: remove logs * fix: presubmit error * fix: lint errors * test: add a unit test for getTransaction * tests: add integration and unit test for getTransaction options properties * fix: lint errors * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * refactor: naming convention for varaible in getTransaction method * refactor: naming convention for varaible in getTransaction method * fix: lint errors * fix: lint error --------- Co-authored-by: Owl Bot * chore(main): release 7.7.0 (#2027) Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> * fix: drop table statement * refactor: delete method for drop statement * Modified delete method to use async await * fix: lint errors * test: unit test for delete method * fix: lint errors * refactor: unit test for drop table * refactor: remove unwanted commented lines * fix: lint errors * refactor: remove unwanted logs and refactor docs of the method * refactor * remove sample file * refactor: unit test for delete method * fix: lint error * refactor: remove unused lines * refactor: remove unused lines * refactor: remove unused lines * fix: lint errors * refactor: remove sample file * refactor * fix: lint errors * fix: lint errors * refactor * fix: lint errors * refactor * refactor * refactor --------- Co-authored-by: surbhigarg92 Co-authored-by: Mend Renovate Co-authored-by: Owl Bot Co-authored-by: gcf-owl-bot[bot] <78513119+gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: danieljbruce Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> --- .gitignore | 1 + src/database.ts | 73 +++++++++++++++++++++++++++++- src/table.ts | 38 +++++++++++++--- test/database.ts | 30 +++++++++++++ test/table.ts | 113 ++++++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 243 insertions(+), 12 deletions(-) diff --git a/.gitignore b/.gitignore index d4f03a0df..14050d4e4 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ system-test/*key.json .DS_Store package-lock.json __pycache__ +.vscode \ No newline at end of file diff --git a/src/database.ts b/src/database.ts index c35f1557e..8c5967604 100644 --- a/src/database.ts +++ b/src/database.ts @@ -133,6 +133,10 @@ export interface SessionPoolConstructor { ): SessionPoolInterface; } +export type GetDatabaseDialectCallback = NormalCallback< + EnumKey +>; + export interface SetIamPolicyRequest { policy: Policy | null; updateMask?: FieldMask | null; @@ -166,7 +170,15 @@ type IDatabaseTranslatedEnum = Omit< typeof databaseAdmin.spanner.admin.database.v1.Database.State >, 'restoreInfo' -> & {restoreInfo?: IRestoreInfoTranslatedEnum | null}; +> & + Omit< + TranslateEnumKeys< + databaseAdmin.spanner.admin.database.v1.IDatabase, + 'databaseDialect', + typeof databaseAdmin.spanner.admin.database.v1.DatabaseDialect + >, + 'restoreInfo' + > & {restoreInfo?: IRestoreInfoTranslatedEnum | null}; /** * IRestoreInfo structure with restore source type enum translated to string form. @@ -312,6 +324,9 @@ class Database extends common.GrpcServiceObject { resourceHeader_: {[k: string]: string}; request: DatabaseRequest; databaseRole?: string | null; + databaseDialect?: EnumKey< + typeof databaseAdmin.spanner.admin.database.v1.DatabaseDialect + > | null; constructor( instance: Instance, name: string, @@ -1476,6 +1491,61 @@ class Database extends common.GrpcServiceObject { return metadata.state || undefined; } + /** + * Retrieves the dialect of the database + * + * @see {@link #getMetadata} + * + * @method Database#getDatabaseDialect + * + * @param {object} [gaxOptions] Request configuration options, + * See {@link https://googleapis.dev/nodejs/google-gax/latest/interfaces/CallOptions.html|CallOptions} + * for more details. + * @param {GetDatabaseDialectCallback} [callback] Callback function. + * @returns {Promise | undefined>} + * When resolved, contains the database dialect of the database if the dialect is defined. + * @example + * const {Spanner} = require('@google-cloud/spanner'); + * const spanner = new Spanner(); + * const instance = spanner.instance('my-instance'); + * const database = instance.database('my-database'); + * const dialect = await database.getDatabaseDialect(); + * const isGoogleSQL = (dialect === 'GOOGLE_STANDARD_SQL'); + * const isPostgreSQL = (dialect === 'POSTGRESQL'); + */ + + getDatabaseDialect( + options?: CallOptions + ): Promise< + | EnumKey + | undefined + >; + getDatabaseDialect(callback: GetDatabaseDialectCallback): void; + getDatabaseDialect( + options: CallOptions, + callback: GetDatabaseDialectCallback + ): void; + async getDatabaseDialect( + optionsOrCallback?: CallOptions | GetDatabaseDialectCallback, + cb?: GetDatabaseDialectCallback + ): Promise< + | EnumKey + | undefined + > { + const gaxOptions = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + + if ( + this.databaseDialect === 'DATABASE_DIALECT_UNSPECIFIED' || + this.databaseDialect === null || + this.databaseDialect === undefined + ) { + const [metadata] = await this.getMetadata(gaxOptions); + this.databaseDialect = metadata.databaseDialect; + } + return this.databaseDialect || undefined; + } + /** * @typedef {array} GetSchemaResponse * @property {string[]} 0 An array of database DDL statements. @@ -3429,6 +3499,7 @@ promisifyAll(Database, { 'batchTransaction', 'getRestoreInfo', 'getState', + 'getDatabaseDialect', 'getOperations', 'runTransaction', 'runTransactionAsync', diff --git a/src/table.ts b/src/table.ts index c737bf96b..7bf85bf4b 100644 --- a/src/table.ts +++ b/src/table.ts @@ -68,6 +68,8 @@ export type UpsertRowsCallback = CommitCallback; export type UpsertRowsResponse = CommitResponse; export type UpsertRowsOptions = MutateRowsOptions; +const GOOGLE_STANDARD_SQL = 'GOOGLE_STANDARD_SQL'; +const POSTGRESQL = 'POSTGRESQL'; /** * Create a Table object to interact with a table in a Cloud Spanner * database. @@ -342,11 +344,37 @@ class Table { const callback = typeof gaxOptionsOrCallback === 'function' ? gaxOptionsOrCallback : cb!; - return this.database.updateSchema( - 'DROP TABLE `' + this.name + '`', - gaxOptions, - callback! - ); + const [schema, table] = this.name.includes('.') + ? this.name.split('.') + : [null, this.name]; + + let dropStatement = 'DROP TABLE `' + table + '`'; + + const performDelete = async (): Promise => { + const result = await this.database.getDatabaseDialect(gaxOptions); + + if (result && result.includes(POSTGRESQL)) { + dropStatement = schema + ? `DROP TABLE "${schema}"."${table}"` + : `DROP TABLE "${table}"`; + } else if (result && result.includes(GOOGLE_STANDARD_SQL)) { + dropStatement = schema + ? 'DROP TABLE `' + schema + '`.`' + table + '`' + : dropStatement; + } + + const updateSchemaResult = callback + ? this.database.updateSchema(dropStatement, gaxOptions, callback) + : this.database.updateSchema(dropStatement, gaxOptions); + + return updateSchemaResult as Promise; + }; + + if (!callback) { + return performDelete() as Promise; + } else { + performDelete(); + } } /** * @typedef {array} DeleteRowsResponse diff --git a/test/database.ts b/test/database.ts index bd697cd9e..1a41f8188 100644 --- a/test/database.ts +++ b/test/database.ts @@ -52,6 +52,7 @@ const fakePfy = extend({}, pfy, { 'batchTransaction', 'getRestoreInfo', 'getState', + 'getDatabaseDialect', 'getOperations', 'runTransaction', 'runTransactionAsync', @@ -2795,6 +2796,35 @@ describe('Database', () => { }); }); + describe('getDatabaseDialect', () => { + it('should get database dialect from database metadata', async () => { + database.getMetadata = async () => [ + {databaseDialect: 'GOOGLE_STANDARD_SQL'}, + ]; + const result = await database.getDatabaseDialect(); + assert.strictEqual(result, 'GOOGLE_STANDARD_SQL'); + }); + + it('should accept and pass gaxOptions to getMetadata', async () => { + const options = {}; + database.getMetadata = async gaxOptions => { + assert.strictEqual(gaxOptions, options); + return [{}]; + }; + await database.getDatabaseDialect(options); + }); + + it('should accept callback and return database dialect', done => { + const databaseDialect = 'GOOGLE_STANDARD_SQL'; + database.getMetadata = async () => [{databaseDialect}]; + database.getDatabaseDialect((err, result) => { + assert.ifError(err); + assert.strictEqual(result, databaseDialect); + done(); + }); + }); + }); + describe('getRestoreInfo', () => { it('should get restore info from database metadata', async () => { const restoreInfo = {sourceType: 'BACKUP'}; diff --git a/test/table.ts b/test/table.ts index c61e8b40f..2886c3287 100644 --- a/test/table.ts +++ b/test/table.ts @@ -67,6 +67,7 @@ describe('Table', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any let TableCached: any; let table; + let tableWithSchema; let transaction: FakeTransaction; const DATABASE = { @@ -75,6 +76,7 @@ describe('Table', () => { }; const NAME = 'table-name'; + const NAMEWITHSCHEMA = 'schema.' + NAME; before(() => { Table = proxyquire('../src/table.js', { @@ -86,6 +88,7 @@ describe('Table', () => { beforeEach(() => { extend(Table, TableCached); table = new Table(DATABASE, NAME); + tableWithSchema = new Table(DATABASE, NAMEWITHSCHEMA); transaction = new FakeTransaction(); }); @@ -209,26 +212,124 @@ describe('Table', () => { }); describe('delete', () => { - it('should update the schema on the database', () => { - const updateSchemaReturnValue = {}; + it('should update the schema on the database for GoogleSQL using await', async () => { + table.database = { + getDatabaseDialect: () => { + return 'GOOGLE_STANDARD_SQL'; + }, + updateSchema: schema => { + assert.strictEqual(schema, 'DROP TABLE `table-name`'); + }, + }; + + await table.delete(); + }); + + it('should update the schema on the database for GoogleSQL using callbacks', () => { + function callback() {} + table.database = { + getDatabaseDialect: () => { + return 'GOOGLE_STANDARD_SQL'; + }, + updateSchema: (schema, gaxOptions, callback_) => { + assert.strictEqual(schema, 'DROP TABLE `table-name`'); + assert.strictEqual(callback_, callback); + }, + }; + table.delete(callback); + }); + + it('should update the schema on the database for GoogleSQL with schema in the table name using await', async () => { + tableWithSchema.database = { + getDatabaseDialect: () => { + return 'GOOGLE_STANDARD_SQL'; + }, + updateSchema: schema => { + assert.strictEqual(schema, 'DROP TABLE `schema`.`table-name`'); + }, + }; + + await tableWithSchema.delete(); + }); + + it('should update the schema on the database for GoogleSQL with schema in the table name using callbacks', () => { + function callback() {} + tableWithSchema.database = { + getDatabaseDialect: () => { + return 'GOOGLE_STANDARD_SQL'; + }, + updateSchema: (schema, gaxOptions, callback_) => { + assert.strictEqual(schema, 'DROP TABLE `schema`.`table-name`'); + assert.strictEqual(callback_, callback); + }, + }; + tableWithSchema.delete(callback); + }); + + it('should update the schema on the database for PostgresSQL using await', async () => { + table.database = { + getDatabaseDialect: () => { + return 'POSTGRESQL'; + }, + updateSchema: schema => { + assert.strictEqual(schema, `DROP TABLE "${table.name}"`); + }, + }; + await table.delete(); + }); + + it('should update the schema on the database for PostgresSQL using callbacks', () => { function callback() {} table.database = { + getDatabaseDialect: () => { + return 'POSTGRESQL'; + }, updateSchema: (schema, gaxOptions, callback_) => { - assert.strictEqual(schema, 'DROP TABLE `' + table.name + '`'); + assert.strictEqual(schema, 'DROP TABLE "table-name"'); assert.strictEqual(callback_, callback); - return updateSchemaReturnValue; }, }; - const returnValue = table.delete(callback); - assert.strictEqual(returnValue, updateSchemaReturnValue); + table.delete(callback); + }); + + it('should update the schema on the database for PostgresSQL with schema in the table name using await', async () => { + tableWithSchema.database = { + getDatabaseDialect: () => { + return 'POSTGRESQL'; + }, + updateSchema: schema => { + assert.strictEqual(schema, `DROP TABLE "schema"."${table.name}"`); + }, + }; + + await tableWithSchema.delete(); + }); + + it('should update the schema on the database for PostgresSQL with schema in the table name using callbacks', () => { + function callback() {} + tableWithSchema.database = { + getDatabaseDialect: () => { + return 'POSTGRESQL'; + }, + updateSchema: (schema, gaxOptions, callback_) => { + assert.strictEqual(schema, `DROP TABLE "schema"."${table.name}"`); + assert.strictEqual(callback_, callback); + }, + }; + + tableWithSchema.delete(callback); }); it('should accept and pass gaxOptions to updateSchema', done => { const gaxOptions = {}; table.database = { + getDatabaseDialect: gaxOptionsFromTable => { + assert.strictEqual(gaxOptionsFromTable, gaxOptions); + return 'GOOGLE_STANDARD_SQL'; + }, updateSchema: (schema, gaxOptionsFromTable) => { assert.strictEqual(gaxOptionsFromTable, gaxOptions); done();