From 39cc5627a73ac8c8a70d637b50b77a156dd27b02 Mon Sep 17 00:00:00 2001 From: Matthew Peveler Date: Wed, 24 Feb 2021 00:14:38 -0500 Subject: [PATCH] ensure semi-colon appears at end of all create scripts (#22) Signed-off-by: Matthew Peveler --- .github/workflows/test.yml | 2 +- spec/db.spec.ts | 74 ++++++++++++++++++++++---------------- spec/utils.spec.ts | 18 +++++++++- src/adapters/mysql.ts | 8 ++--- src/adapters/postgresql.ts | 8 ++--- src/adapters/sqlite.ts | 5 +-- src/adapters/sqlserver.ts | 10 +++--- src/utils.ts | 8 +++++ 8 files changed, 85 insertions(+), 48 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ab2ffa4..57cd356 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -30,7 +30,7 @@ jobs: sqlserver-collation: Latin1_General_CS_AS - node-version: 14.x mysql-version: 8 - maraidb-version: 10 + maraidb-version: 10.5 cassandra-version: 3 postgres-version: 13 sqlserver-collation: Latin1_General_CI_AS diff --git a/spec/db.spec.ts b/spec/db.spec.ts index a274d3a..ebac930 100644 --- a/spec/db.spec.ts +++ b/spec/db.spec.ts @@ -353,7 +353,7 @@ describe('db', () => { const [createScript] = await dbConn.getTableCreateScript('users'); if (dbAdapter === 'mysql' && versionCompare(dbConn.getVersion().version, '8') >= 0) { - expect(createScript).to.contain('CREATE TABLE `users` (\n' + + expect(createScript).to.eql('CREATE TABLE `users` (\n' + ' `id` int NOT NULL AUTO_INCREMENT,\n' + ' `username` varchar(45) DEFAULT NULL,\n' + ' `email` varchar(150) DEFAULT NULL,\n' + @@ -363,9 +363,10 @@ describe('db', () => { ' PRIMARY KEY (`id`),\n' + ' KEY `role_id` (`role_id`),\n' + ' CONSTRAINT `users_ibfk_1` FOREIGN KEY (`role_id`) REFERENCES `roles` (`id`) ON DELETE CASCADE\n' + - ') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci'); + ') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci;'); } else if (mysqlAdapters.includes(dbAdapter)) { - expect(createScript).to.contain('CREATE TABLE `users` (\n' + + const charset = dbAdapter === 'mariadb' && versionCompare(dbConn.getVersion().version, '10.1') > 0 ? 'utf8mb4' : 'latin1'; + expect(createScript).to.eql('CREATE TABLE `users` (\n' + ' `id` int(11) NOT NULL AUTO_INCREMENT,\n' + ' `username` varchar(45) DEFAULT NULL,\n' + ' `email` varchar(150) DEFAULT NULL,\n' + @@ -375,9 +376,10 @@ describe('db', () => { ' PRIMARY KEY (`id`),\n' + ' KEY `role_id` (`role_id`),\n' + ' CONSTRAINT `users_ibfk_1` FOREIGN KEY (`role_id`) REFERENCES `roles` (`id`) ON DELETE CASCADE\n' + - ') ENGINE=InnoDB'); + `) ENGINE=InnoDB DEFAULT CHARSET=${charset};`); } else if (postgresAdapters.includes(dbAdapter)) { - expect(createScript).to.eql('CREATE TABLE public.users (\n' + + expect(createScript).to.eql( + 'CREATE TABLE public.users (\n' + ' id integer NOT NULL,\n' + ' username text NOT NULL,\n' + ' email text NOT NULL,\n' + @@ -386,19 +388,21 @@ describe('db', () => { ' createdat date NULL\n' + ');\n' + '\n' + - 'ALTER TABLE public.users ADD CONSTRAINT users_pkey PRIMARY KEY (id)', + 'ALTER TABLE public.users ADD CONSTRAINT users_pkey PRIMARY KEY (id);', ); } else if (dbAdapter === 'sqlserver') { - expect(createScript).to.contain('CREATE TABLE users (\r\n' + - ' id int IDENTITY(1,1) NOT NULL,\r\n' + - ' username varchar(45) NULL,\r\n' + - ' email varchar(150) NULL,\r\n' + - ' password varchar(45) NULL,\r\n' + - ' role_id int NULL,\r\n' + - ' createdat datetime NULL,\r\n' + - ')\r\n'); - expect(createScript).to.contain('ALTER TABLE users ADD CONSTRAINT PK__users'); - expect(createScript).to.contain('PRIMARY KEY (id)'); + expect(createScript).to.match(new RegExp( + 'CREATE TABLE users \\(\\r\\n' + + ' id int IDENTITY\\(1,1\\) NOT NULL,\\r\\n' + + ' username varchar\\(45\\) NULL,\\r\\n' + + ' email varchar\\(150\\) NULL,\\r\\n' + + ' password varchar\\(45\\) NULL,\\r\\n' + + ' role_id int NULL,\\r\\n' + + ' createdat datetime NULL,\\r\\n' + + '\\);\\r\\n' + + '\\r\\n' + + 'ALTER TABLE users ADD CONSTRAINT PK__users__[a-zA-Z0-9]+ PRIMARY KEY \\(id\\);' + )); } else if (dbAdapter === 'sqlite') { expect(createScript).to.eql('CREATE TABLE users (\n' + ' id INTEGER NOT NULL,\n' + @@ -408,7 +412,8 @@ describe('db', () => { ' role_id INT,\n' + ' createdat DATETIME NULL,\n' + ' PRIMARY KEY (id),\n' + - ' FOREIGN KEY (role_id) REFERENCES roles (id)\n)', + ' FOREIGN KEY (role_id) REFERENCES roles (id)\n' + + ');', ); } else if (dbAdapter === 'cassandra') { expect(createScript).to.eql(undefined); @@ -569,10 +574,11 @@ describe('db', () => { it('should return CREATE VIEW script', async () => { const [createScript] = await dbConn.getViewCreateScript('email_view'); if (mysqlAdapters.includes(dbAdapter)) { - expect(createScript).to.contain([ + expect(createScript).to.eql([ + 'CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER', 'VIEW `email_view`', 'AS select `users`.`email` AS `email`,`users`.`password` AS `password`', - 'from `users`', + 'from `users`;', ].join(' ')); } else if (dbAdapter === 'postgresql') { expect(createScript).to.eql([ @@ -589,15 +595,15 @@ describe('db', () => { ].join('\n')); } else if (dbAdapter === 'sqlserver') { expect(createScript).to.eql([ - '\nCREATE VIEW dbo.email_view AS', + 'CREATE VIEW dbo.email_view AS', 'SELECT dbo.users.email, dbo.users.password', - 'FROM dbo.users;\n', + 'FROM dbo.users;', ].join('\n')); } else if (dbAdapter === 'sqlite') { expect(createScript).to.eql([ 'CREATE VIEW email_view AS', ' SELECT users.email, users.password', - ' FROM users', + ' FROM users;', ].join('\n')); } else if (dbAdapter === 'cassandra') { expect(createScript).to.eql(undefined); @@ -611,12 +617,11 @@ describe('db', () => { it('should return CREATE PROCEDURE/FUNCTION script', async () => { const [createScript] = await dbConn.getRoutineCreateScript('users_count', 'Procedure'); if (mysqlAdapters.includes(dbAdapter)) { - expect(createScript).to.contain('CREATE DEFINER='); - expect(createScript).to.contain([ - 'PROCEDURE `users_count`()', + expect(createScript).to.eql([ + 'CREATE DEFINER=`root`@`localhost` PROCEDURE `users_count`()', 'BEGIN', ' SELECT COUNT(*) FROM users;', - 'END', + 'END;', ].join('\n')); } else if (dbAdapter === 'postgresql') { expect(createScript).to.eql([ @@ -625,19 +630,26 @@ describe('db', () => { ' LANGUAGE sql', 'AS $function$', ' SELECT COUNT(*) FROM users AS total;', - '$function$\n', + '$function$;', ].join('\n')); } else if (dbAdapter === 'redshift') { expect(createScript).to.eql([ 'CREATE OR REPLACE FUNCTION public.users_count()', ' RETURNS bigint AS $$', ' SELECT COUNT(*) FROM users AS total;', - '$$ LANGUAGE sql VOLATILE', + '$$ LANGUAGE sql VOLATILE;', ].join('\n')); } else if (dbAdapter === 'sqlserver') { - expect(createScript).to.contain('CREATE PROCEDURE dbo.users_count'); - expect(createScript).to.contain('@Count int OUTPUT'); - expect(createScript).to.contain('SELECT @Count = COUNT(*) FROM dbo.users'); + expect(createScript).to.eql([ + 'CREATE PROCEDURE dbo.users_count', + '(', + ' @Count int OUTPUT', + ')', + 'AS', + ' BEGIN', + ' SELECT @Count = COUNT(*) FROM dbo.users', + ' END;' + ].join('\n')); } else if (dbAdapter === 'cassandra' || dbAdapter === 'sqlite') { expect(createScript).to.eql(undefined); } else { diff --git a/spec/utils.spec.ts b/spec/utils.spec.ts index c05a68a..0abb428 100644 --- a/spec/utils.spec.ts +++ b/spec/utils.spec.ts @@ -1,5 +1,5 @@ import { expect } from 'chai'; -import { versionCompare } from '../src/utils'; +import { appendSemiColon, versionCompare } from '../src/utils'; describe('utils', () => { describe('.versionCompare', () => { @@ -23,4 +23,20 @@ describe('utils', () => { }); }); }); + + describe('.appendSemiColon', () => { + const parameters: [string][] = [ + ['test'], + ['test;'], + ['\ntest'], + ['test\n'], + ['\ntest\n'], + ['\ntest;\n'], + ]; + parameters.forEach(([inputString]) => { + it(`.appendSemiColon(${JSON.stringify(inputString)}) === 'test;'`, () => { + expect(appendSemiColon(inputString)).to.eql('test;'); + }); + }) + }); }); diff --git a/src/adapters/mysql.ts b/src/adapters/mysql.ts index 0ee70d5..987f8d7 100644 --- a/src/adapters/mysql.ts +++ b/src/adapters/mysql.ts @@ -2,7 +2,7 @@ import mysql from 'mysql2'; import { identify } from 'sql-query-identifier'; import createLogger from '../logger'; -import { createCancelablePromise } from '../utils'; +import { appendSemiColon, createCancelablePromise } from '../utils'; import { AbstractAdapter } from './abstract_adapter'; import type { Result } from 'sql-query-identifier'; @@ -388,7 +388,7 @@ export default class MysqlAdapter extends AbstractAdapter { const { data } = await this.driverExecuteQuery({ query: sql }); - return (data).map((row) => row['Create Table'] as string); + return (data).map((row) => appendSemiColon(row['Create Table'] as string)); } async getViewCreateScript(view: string): Promise { @@ -396,7 +396,7 @@ export default class MysqlAdapter extends AbstractAdapter { const { data } = await this.driverExecuteQuery({ query: sql }); - return (data).map((row) => row['Create View'] as string); + return (data).map((row) => appendSemiColon(row['Create View'] as string)); } async getRoutineCreateScript(routine: string, type: string): Promise { @@ -404,7 +404,7 @@ export default class MysqlAdapter extends AbstractAdapter { const { data } = await this.driverExecuteQuery({ query: sql }); - return (data).map((row) => row[`Create ${type}`] as string); + return (data).map((row) => appendSemiColon(row[`Create ${type}`] as string)); } async getSchema(connection?: mysql.PoolConnection): Promise { diff --git a/src/adapters/postgresql.ts b/src/adapters/postgresql.ts index 491dea9..eb7195a 100644 --- a/src/adapters/postgresql.ts +++ b/src/adapters/postgresql.ts @@ -3,7 +3,7 @@ import { identify } from 'sql-query-identifier'; import { buildDatabaseFilter, buildSchemaFilter } from '../filters'; import createLogger from '../logger'; -import { createCancelablePromise, versionCompare } from '../utils'; +import { appendSemiColon, createCancelablePromise, versionCompare } from '../utils'; import { Adapter, ADAPTERS } from './'; import { AbstractAdapter, QueryArgs, QueryRowResult, TableKeysResult } from './abstract_adapter'; @@ -398,7 +398,7 @@ export default class PostgresqlAdapter extends AbstractAdapter { params, })).rows[0]; if (constraintResult.constraint.length > 0) { - createTable += `\n${constraintResult.constraint}`; + createTable += `\n${constraintResult.constraint};`; } return [createTable]; } @@ -430,7 +430,7 @@ export default class PostgresqlAdapter extends AbstractAdapter { WHERE proname = $1 AND n.nspname = $2 `; - mapFunction = (row: {[column: string]: unknown}): string => row.pg_get_functiondef as string; + mapFunction = (row: {[column: string]: unknown}): string => appendSemiColon(row.pg_get_functiondef as string); } else { // -- pg_catalog.array_to_string(p.proacl, '\n') AS "Access privileges", sql = ` @@ -477,7 +477,7 @@ export default class PostgresqlAdapter extends AbstractAdapter { (val: string, idx: number) => `${(row.proargnames as string[])[idx]} ${val}` ).join(', '); } - return `CREATE OR REPLACE FUNCTION ${row.nspname as string}.${row.proname as string}(${args})\n RETURNS ${row.prorettype as string} AS $$${row.prosrc as string}$$ LANGUAGE ${row.lanname as string} ${row.provolatility as string}`; + return `CREATE OR REPLACE FUNCTION ${row.nspname as string}.${row.proname as string}(${args})\n RETURNS ${row.prorettype as string} AS $$${row.prosrc as string}$$ LANGUAGE ${row.lanname as string} ${row.provolatility as string};`; }; } diff --git a/src/adapters/sqlite.ts b/src/adapters/sqlite.ts index b665dee..ef7b876 100644 --- a/src/adapters/sqlite.ts +++ b/src/adapters/sqlite.ts @@ -2,6 +2,7 @@ import sqlite3 from 'sqlite3'; import { identify, Result } from 'sql-query-identifier'; import createLogger from '../logger'; +import { appendSemiColon } from '../utils'; import { Adapter, ADAPTERS } from './'; import { AbstractAdapter } from './abstract_adapter'; @@ -187,7 +188,7 @@ export default class SqliteAdapter extends AbstractAdapter { const { data } = await this.driverExecuteQuery({ query: sql }); - return (<{sql: string}[]>data).map((row) => row.sql); + return (<{sql: string}[]>data).map((row) => appendSemiColon(row.sql)); } async getViewCreateScript(view: string): Promise { @@ -199,7 +200,7 @@ export default class SqliteAdapter extends AbstractAdapter { const { data } = await this.driverExecuteQuery({ query: sql }); - return (<{sql: string}[]>data).map((row) => row.sql); + return (<{sql: string}[]>data).map((row) => appendSemiColon(row.sql)); } async truncateAllTables(): Promise { diff --git a/src/adapters/sqlserver.ts b/src/adapters/sqlserver.ts index 0c7c4d3..5cdd0d6 100644 --- a/src/adapters/sqlserver.ts +++ b/src/adapters/sqlserver.ts @@ -2,7 +2,7 @@ import { ConnectionPool } from 'mssql'; import { buildDatabaseFilter, buildSchemaFilter } from '../filters'; import createLogger from '../logger'; -import { identifyCommands } from '../utils'; +import { identifyCommands, appendSemiColon } from '../utils'; import { AbstractAdapter, QueryArgs, QueryRowResult } from './abstract_adapter'; import type { config, Request, IResult, IRecordSet } from 'mssql'; @@ -298,11 +298,11 @@ export default class SqlServerAdapter extends AbstractAdapter { const sql = ` SELECT ('CREATE TABLE ' + so.name + ' (' + CHAR(13)+CHAR(10) + REPLACE(o.list, ' ', CHAR(13)) + - ')' + CHAR(13)+CHAR(10) + + ');' + CHAR(13)+CHAR(10) + CASE WHEN tc.constraint_name IS NULL THEN '' ELSE + CHAR(13)+CHAR(10) + 'ALTER TABLE ' + so.Name + ' ADD CONSTRAINT ' + tc.constraint_name + - ' PRIMARY KEY ' + '(' + LEFT(j.list, Len(j.list)-1) + ')' + ' PRIMARY KEY ' + '(' + LEFT(j.list, Len(j.list)-1) + ');' END) AS createtable FROM sysobjects so CROSS APPLY @@ -369,7 +369,7 @@ export default class SqlServerAdapter extends AbstractAdapter { const { data } = await this.driverExecuteSingleQuery<{ViewDefinition: string}>({ query: sql }); - return data.map((row) => row.ViewDefinition); + return data.map((row) => appendSemiColon(row.ViewDefinition)); } async getRoutineCreateScript(routine: string): Promise { @@ -381,7 +381,7 @@ export default class SqlServerAdapter extends AbstractAdapter { const { data } = await this.driverExecuteSingleQuery<{routine_definition: string}>({ query: sql }); - return data.map((row) => row.routine_definition); + return data.map((row) => appendSemiColon(row.routine_definition)); } async truncateAllTables(): Promise { diff --git a/src/utils.ts b/src/utils.ts index 7e13100..e35ed72 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -94,3 +94,11 @@ export function identifyCommands(queryText: string): Result[] { return []; } } + +export function appendSemiColon(query: string): string { + let result = query.trim() + if (result[result.length - 1] !== ';') { + result += ';'; + } + return result; +}