From 52d19404608ecbeb940e54fc0609efd32fc7ed69 Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Tue, 9 Mar 2021 16:12:10 +0000 Subject: [PATCH] Removed queries against information_schema (#12739) no-issue We were originally checking the state of the database, e.g. if a foreign key constraint existed, so that we could conditionally act upon it. This was to ensure that our migrations are idempotent. Some database configurations, for example if you have many databases on a single MySQL instance, would cause these information_schema queries to take an exceptionally long time. In order to speed up migrations, we instead attempt the action we want to apply to the database, and then catch relevant errors to ensure the migration is idempotent. SQLite does not error when adding duplicate foreign or primary key constraints, meaning that we must keep in pre-checks for these operations, when running on SQLite Co-authored-by: Daniel Lockyer --- core/server/data/schema/commands.js | 226 +++++++++++++--------------- 1 file changed, 101 insertions(+), 125 deletions(-) diff --git a/core/server/data/schema/commands.js b/core/server/data/schema/commands.js index 1c3629c43a..68a6461190 100644 --- a/core/server/data/schema/commands.js +++ b/core/server/data/schema/commands.js @@ -63,64 +63,30 @@ function dropColumn(tableName, column, transaction) { }); } -/** - * Checks if unique index exists in a table over the given columns. - * - * @param {string} tableName - name of the table to add unique constraint to - * @param {string|[string]} columns - column(s) to form unique constraint with - * @param {Object} transaction - connection object containing knex reference - * @param {Object} transaction.knex - knex instance - */ -async function hasUnique(tableName, columns, transaction) { - const knex = (transaction || db.knex); - const client = knex.client.config.client; - const columnNames = _.isArray(columns) ? columns.join('_') : columns; - const constraintName = `${tableName}_${columnNames}_unique`; - - if (client === 'mysql') { - const dbName = knex.client.config.connection.database; - const [rawConstraints] = await knex.raw(` - SELECT CONSTRAINT_NAME - FROM information_schema.TABLE_CONSTRAINTS - WHERE 1=1 - AND CONSTRAINT_SCHEMA=:dbName - AND TABLE_NAME=:tableName - AND CONSTRAINT_TYPE='UNIQUE'`, {dbName, tableName}); - const dbConstraints = rawConstraints.map(c => c.CONSTRAINT_NAME); - - if (dbConstraints.includes(constraintName)) { - return true; - } - } else { - const rawConstraints = await knex.raw(`PRAGMA index_list('${tableName}');`); - const dbConstraints = rawConstraints.map(c => c.name); - - if (dbConstraints.includes(constraintName)) { - return true; - } - } - - return false; -} - /** * Adds an unique index to a table over the given columns. * * @param {string} tableName - name of the table to add unique constraint to * @param {string|[string]} columns - column(s) to form unique constraint with - * @param {Object} transaction - connection object containing knex reference - * @param {Object} transaction.knex - knex instance + * @param {import('knex')} transaction - connection object containing knex reference */ async function addUnique(tableName, columns, transaction) { - const hasUniqueConstraint = await hasUnique(tableName, columns, transaction); - - if (!hasUniqueConstraint) { + try { logging.info(`Adding unique constraint for: ${columns} in table ${tableName}`); - return (transaction || db.knex).schema.table(tableName, function (table) { + + return await (transaction || db.knex).schema.table(tableName, function (table) { table.unique(columns); }); - } else { - logging.warn(`Constraint for: ${columns} already exists for table: ${tableName}`); + } catch (err) { + if (err.code === 'SQLITE_ERROR') { + logging.warn(`Constraint for: ${columns} already exists for table: ${tableName}`); + return; + } + if (err.code === 'ER_DUP_KEYNAME') { + logging.warn(`Constraint for: ${columns} already exists for table: ${tableName}`); + return; + } + throw err; } } @@ -129,19 +95,25 @@ async function addUnique(tableName, columns, transaction) { * * @param {string} tableName - name of the table to drop unique constraint from * @param {string|[string]} columns - column(s) unique constraint was formed - * @param {Object} transaction - connection object containing knex reference - * @param {Object} transaction.knex - knex instance + * @param {import('knex')} transaction - connection object containing knex reference */ async function dropUnique(tableName, columns, transaction) { - const hasUniqueConstraint = await hasUnique(tableName, columns, transaction); - - if (hasUniqueConstraint) { + try { logging.info(`Dropping unique constraint for: ${columns} in table: ${tableName}`); - return (transaction || db.knex).schema.table(tableName, function (table) { + + return await (transaction || db.knex).schema.table(tableName, function (table) { table.dropUnique(columns); }); - } else { - logging.warn(`Constraint for: ${columns} does not exist for table: ${tableName}`); + } catch (err) { + if (err.code === 'SQLITE_ERROR') { + logging.warn(`Constraint for: ${columns} does not exist for table: ${tableName}`); + return; + } + if (err.code === 'ER_CANT_DROP_FIELD_OR_KEY') { + logging.warn(`Constraint for: ${columns} does not exist for table: ${tableName}`); + return; + } + throw err; } } @@ -153,34 +125,21 @@ async function dropUnique(tableName, columns, transaction) { * @param {string} configuration.fromColumn - column of the table to add the foreign key to * @param {string} configuration.toTableName - name of the table to point the foreign key to * @param {string} configuration.toColumn - column of the table to point the foreign key to - * @param {Object} configuration.transaction - connection object containing knex reference - * @param {Object} configuration.transaction.knex - knex instance + * @param {import('knex')} configuration.transaction - connection object containing knex reference */ -async function hasForeign({fromTable, fromColumn, toTable, toColumn, transaction}) { +async function hasForeignSQLite({fromTable, fromColumn, toTable, toColumn, transaction}) { const knex = (transaction || db.knex); const client = knex.client.config.client; - if (client === 'mysql') { - const dbName = knex.client.config.connection.database; - const [rawConstraints] = await knex.raw(` - SELECT i.TABLE_NAME, k.COLUMN_NAME, k.REFERENCED_TABLE_NAME, k.REFERENCED_COLUMN_NAME - FROM information_schema.TABLE_CONSTRAINTS i - INNER JOIN information_schema.KEY_COLUMN_USAGE k ON i.CONSTRAINT_NAME = k.CONSTRAINT_NAME - WHERE i.CONSTRAINT_TYPE = 'FOREIGN KEY' - AND i.CONSTRAINT_SCHEMA=:dbName - AND i.TABLE_NAME = :fromTable - AND k.COLUMN_NAME = :fromColumn - AND k.REFERENCED_TABLE_NAME = :toTable - AND k.REFERENCED_COLUMN_NAME = :toColumn`, {dbName, fromTable, fromColumn, toTable, toColumn}); - - return rawConstraints.length >= 1; - } else { - const foreignKeys = await knex.raw(`PRAGMA foreign_key_list('${fromTable}');`); - - const hasForeignKey = foreignKeys.some(foreignKey => foreignKey.table === toTable && foreignKey.from === fromColumn && foreignKey.to === toColumn); - - return hasForeignKey; + if (client !== 'sqlite3') { + throw new Error('Must use hasForeignSQLite3 on an SQLite3 database'); } + + const foreignKeys = await knex.raw(`PRAGMA foreign_key_list('${fromTable}');`); + + const hasForeignKey = foreignKeys.some(foreignKey => foreignKey.table === toTable && foreignKey.from === fromColumn && foreignKey.to === toColumn); + + return hasForeignKey; } /** @@ -192,18 +151,23 @@ async function hasForeign({fromTable, fromColumn, toTable, toColumn, transaction * @param {string} configuration.toTableName - name of the table to point the foreign key to * @param {string} configuration.toColumn - column of the table to point the foreign key to * @param {Boolean} configuration.cascadeDelete - adds the "on delete cascade" option if true - * @param {Object} configuration.transaction - connection object containing knex reference - * @param {Object} configuration.transaction.knex - knex instance + * @param {import('knex')} configuration.transaction - connection object containing knex reference */ async function addForeign({fromTable, fromColumn, toTable, toColumn, cascadeDelete = false, transaction}) { - const hasForeignKey = await hasForeign({fromTable, fromColumn, toTable, toColumn, transaction}); - - if (!hasForeignKey) { + const isSQLite = db.knex.client.config.client === 'sqlite3'; + if (isSQLite) { + const foreignKeyExists = await hasForeignSQLite({fromTable, fromColumn, toTable, toColumn, transaction}); + if (foreignKeyExists) { + logging.warn(`Skipped adding foreign key from ${fromTable}.${fromColumn} to ${toTable}.${toColumn} - foreign key already exists`); + return; + } + } + try { logging.info(`Adding foreign key from ${fromTable}.${fromColumn} to ${toTable}.${toColumn}`); //disable and re-enable foreign key checks on sqlite because of https://github.com/knex/knex/issues/4155 let foreignKeysEnabled; - if (db.knex.client.config.client === 'sqlite3') { + if (isSQLite) { foreignKeysEnabled = await db.knex.raw('PRAGMA foreign_keys;'); if (foreignKeysEnabled[0].foreign_keys) { await db.knex.raw('PRAGMA foreign_keys = OFF;'); @@ -218,13 +182,17 @@ async function addForeign({fromTable, fromColumn, toTable, toColumn, cascadeDele } }); - if (db.knex.client.config.client === 'sqlite3') { + if (isSQLite) { if (foreignKeysEnabled[0].foreign_keys) { await db.knex.raw('PRAGMA foreign_keys = ON;'); } } - } else { - logging.warn(`Skipped adding foreign key from ${fromTable}.${fromColumn} to ${toTable}.${toColumn} - foreign key already exists`); + } catch (err) { + if (err.code === 'ER_DUP_KEY') { + logging.warn(`Skipped adding foreign key from ${fromTable}.${fromColumn} to ${toTable}.${toColumn} - foreign key already exists`); + return; + } + throw err; } } @@ -236,18 +204,23 @@ async function addForeign({fromTable, fromColumn, toTable, toColumn, cascadeDele * @param {string} configuration.fromColumn - column of the table to add the foreign key to * @param {string} configuration.toTableName - name of the table to point the foreign key to * @param {string} configuration.toColumn - column of the table to point the foreign key to - * @param {Object} configuration.transaction - connection object containing knex reference - * @param {Object} configuration.transaction.knex - knex instance + * @param {import('knex')} configuration.transaction - connection object containing knex reference */ async function dropForeign({fromTable, fromColumn, toTable, toColumn, transaction}) { - const hasForeignKey = await hasForeign({fromTable, fromColumn, toTable, toColumn, transaction}); - - if (hasForeignKey) { + const isSQLite = db.knex.client.config.client === 'sqlite3'; + if (isSQLite) { + const foreignKeyExists = await hasForeignSQLite({fromTable, fromColumn, toTable, toColumn, transaction}); + if (!foreignKeyExists) { + logging.warn(`Skipped dropping foreign key from ${fromTable}.${fromColumn} to ${toTable}.${toColumn} - foreign key does not exist`); + return; + } + } + try { logging.info(`Dropping foreign key from ${fromTable}.${fromColumn} to ${toTable}.${toColumn}`); //disable and re-enable foreign key checks on sqlite because of https://github.com/knex/knex/issues/4155 let foreignKeysEnabled; - if (db.knex.client.config.client === 'sqlite3') { + if (isSQLite) { foreignKeysEnabled = await db.knex.raw('PRAGMA foreign_keys;'); if (foreignKeysEnabled[0].foreign_keys) { await db.knex.raw('PRAGMA foreign_keys = OFF;'); @@ -258,44 +231,38 @@ async function dropForeign({fromTable, fromColumn, toTable, toColumn, transactio table.dropForeign(fromColumn); }); - if (db.knex.client.config.client === 'sqlite3') { + if (isSQLite) { if (foreignKeysEnabled[0].foreign_keys) { await db.knex.raw('PRAGMA foreign_keys = ON;'); } } - } else { - logging.warn(`Skipped dropping foreign key from ${fromTable}.${fromColumn} to ${toTable}.${toColumn} - foreign key does not exist`); + } catch (err) { + if (err.code === 'ER_CANT_DROP_FIELD_OR_KEY') { + logging.warn(`Skipped dropping foreign key from ${fromTable}.${fromColumn} to ${toTable}.${toColumn} - foreign key does not exist`); + return; + } + throw err; } } /** * Checks if primary key index exists in a table over the given columns. * - * @param {string} tableName - name of the table to add primary key constraint to - * @param {Object} transaction - connnection object containing knex reference - * @param {Object} transaction.knex - knex instance + * @param {string} tableName - name of the table to check primary key constraint on + * @param {import('knex')} transaction - connnection object containing knex reference */ -async function hasPrimaryKey(tableName, transaction) { +async function hasPrimaryKeySQLite(tableName, transaction) { const knex = (transaction || db.knex); const client = knex.client.config.client; - if (client === 'mysql') { - const dbName = knex.client.config.connection.database; - const [rawConstraints] = await knex.raw(` - SELECT CONSTRAINT_NAME - FROM information_schema.TABLE_CONSTRAINTS - WHERE 1=1 - AND CONSTRAINT_SCHEMA=:dbName - AND TABLE_NAME=:tableName - AND CONSTRAINT_TYPE='PRIMARY KEY'`, {dbName, tableName}); - - return rawConstraints.length > 0; - } else { - const rawConstraints = await knex.raw(`PRAGMA index_list('${tableName}');`); - const tablePrimaryKey = rawConstraints.find(c => c.origin === 'pk'); - - return tablePrimaryKey; + if (client !== 'sqlite3') { + throw new Error('Must use hasPrimaryKeySQLite on an SQLite3 database'); } + + const rawConstraints = await knex.raw(`PRAGMA index_list('${tableName}');`); + const tablePrimaryKey = rawConstraints.find(c => c.origin === 'pk'); + + return tablePrimaryKey; } /** @@ -303,19 +270,28 @@ async function hasPrimaryKey(tableName, transaction) { * * @param {string} tableName - name of the table to add primaykey constraint to * @param {string|[string]} columns - column(s) to form primary key constraint with - * @param {Object} transaction - connnection object containing knex reference - * @param {Object} transaction.knex - knex instance + * @param {import('knex')} transaction - connnection object containing knex reference */ async function addPrimaryKey(tableName, columns, transaction) { - const hasUniqueConstraint = await hasPrimaryKey(tableName, transaction); - - if (!hasUniqueConstraint) { + const isSQLite = db.knex.client.config.client === 'sqlite3'; + if (isSQLite) { + const primaryKeyExists = await hasPrimaryKeySQLite(tableName, transaction); + if (primaryKeyExists) { + logging.warn(`Primary key constraint for: ${columns} already exists for table: ${tableName}`); + return; + } + } + try { logging.info(`Adding primary key constraint for: ${columns} in table ${tableName}`); - return (transaction || db.knex).schema.table(tableName, function (table) { + return await (transaction || db.knex).schema.table(tableName, function (table) { table.primary(columns); }); - } else { - logging.warn(`Primary key constraint for: ${columns} already exists for table: ${tableName}`); + } catch (err) { + if (err.code === 'ER_MULTIPLE_PRI_KEY') { + logging.warn(`Primary key constraint for: ${columns} already exists for table: ${tableName}`); + return; + } + throw err; } }