Add primary/foreign support to SQLite on alterTable (#4162)

Co-authored-by: Igor Savin <iselwin@gmail.com>
This commit is contained in:
Rijk van Zanten 2020-12-26 12:10:40 -05:00 committed by GitHub
parent f6a64afaed
commit 9692e36561
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 406 additions and 9 deletions

View File

@ -29,8 +29,16 @@ const POOL_CONFIG_OPTIONS = Object.freeze([
'Promise',
]);
/**
* Regex that only matches comma's in strings that aren't wrapped in parentheses. Can be used to
* safely split strings like `id int, name string, body text, primary key (id, name)` into definition
* rows
*/
const COMMA_NO_PAREN_REGEX = /,[\s](?![^(]*\))/g;
module.exports = {
CLIENT_ALIASES,
SUPPORTED_CLIENTS,
POOL_CONFIG_OPTIONS,
COMMA_NO_PAREN_REGEX,
};

View File

@ -14,6 +14,7 @@ const isEmpty = require('lodash/isEmpty');
const negate = require('lodash/negate');
const omit = require('lodash/omit');
const uniqueId = require('lodash/uniqueId');
const { COMMA_NO_PAREN_REGEX } = require('../../../constants');
// So altering the schema in SQLite3 is a major pain.
// We have our own object to deal with the renaming and altering the types
@ -328,7 +329,7 @@ assign(SQLite3_DDL.prototype, {
}
const updatedDefs = defs
.split(',')
.split(COMMA_NO_PAREN_REGEX)
.map((line) => line.trim())
.filter((defLine) => {
if (
@ -359,6 +360,157 @@ assign(SQLite3_DDL.prototype, {
);
},
dropPrimary: async function (constraintName) {
return this.client.transaction(
async (trx) => {
this.trx = trx;
const sql = await this.getTableSql();
const createTable = sql[0];
const oneLineSql = createTable.sql.replace(/\s+/g, ' ');
const matched = oneLineSql.match(/^CREATE TABLE\s+(\S+)\s*\((.*)\)/);
const defs = matched[2];
if (!defs) {
throw new Error('No column definitions in this statement!');
}
const updatedDefs = defs
.split(COMMA_NO_PAREN_REGEX)
.map((line) => line.trim())
.filter((defLine) => {
if (
defLine.startsWith('constraint') === false &&
defLine.includes('primary key') === false
)
return true;
if (constraintName) {
if (defLine.includes(constraintName)) return false;
return true;
} else {
return true;
}
})
.join(', ');
const newSql = oneLineSql.replace(defs, updatedDefs);
return this.reinsertMapped(createTable, newSql, (row) => {
return row;
});
},
{ connection: this.connection }
);
},
primary: async function (columns, constraintName) {
return this.client.transaction(
async (trx) => {
this.trx = trx;
const tableInfo = (await this.getTableSql())[0];
const currentSQL = tableInfo.sql;
const oneLineSQL = currentSQL.replace(/\s+/g, ' ');
const matched = oneLineSQL.match(/^CREATE TABLE\s+(\S+)\s*\((.*)\)/);
const columnDefinitions = matched[2];
if (!columnDefinitions) {
throw new Error('No column definitions in this statement!');
}
const primaryKeyDef = `primary key(${columns.join(',')})`;
const constraintDef = constraintName
? `constraint ${constraintName} ${primaryKeyDef}`
: primaryKeyDef;
const newColumnDefinitions = [
...columnDefinitions
.split(COMMA_NO_PAREN_REGEX)
.map((line) => line.trim())
.filter((line) => line.startsWith('primary') === false)
.map((line) => line.replace(/primary key/i, '')),
constraintDef,
].join(', ');
const newSQL = oneLineSQL.replace(
columnDefinitions,
newColumnDefinitions
);
return this.reinsertMapped(tableInfo, newSQL, (row) => {
return row;
});
},
{ connection: this.connection }
);
},
foreign: async function (foreignInfo) {
return this.client.transaction(
async (trx) => {
this.trx = trx;
const tableInfo = (await this.getTableSql())[0];
const currentSQL = tableInfo.sql;
const oneLineSQL = currentSQL.replace(/\s+/g, ' ');
const matched = oneLineSQL.match(/^CREATE TABLE\s+(\S+)\s*\((.*)\)/);
const columnDefinitions = matched[2];
if (!columnDefinitions) {
throw new Error('No column definitions in this statement!');
}
const newColumnDefinitions = columnDefinitions
.split(COMMA_NO_PAREN_REGEX)
.map((line) => line.trim());
let newForeignSQL = '';
if (foreignInfo.keyName) {
newForeignSQL += `CONSTRAINT ${foreignInfo.keyName}`;
}
newForeignSQL += ` FOREIGN KEY (${foreignInfo.column.join(', ')}) `;
newForeignSQL += ` REFERENCES ${foreignInfo.inTable} (${foreignInfo.references})`;
if (foreignInfo.onUpdate) {
newForeignSQL += ` ON UPDATE ${foreignInfo.onUpdate}`;
}
if (foreignInfo.onDelete) {
newForeignSQL += ` ON DELETE ${foreignInfo.onUpdate}`;
}
newColumnDefinitions.push(newForeignSQL);
const newSQL = oneLineSQL.replace(
columnDefinitions,
newColumnDefinitions.join(', ')
);
return this.reinsertMapped(tableInfo, newSQL, (row) => {
return row;
});
},
{ connection: this.connection }
);
},
/**
* @fixme
*
* There's a bunch of overlap between renameColumn/dropColumn/dropForeign/primary/foreign.
* It'll be helpful to refactor this file heavily to combine/optimize some of these calls
*/
reinsertMapped(createTable, newSql, mapRow) {
return Promise.resolve()
.then(() => this.createTempTable(createTable))

View File

@ -64,6 +64,20 @@ TableCompiler_SQLite3.prototype.dropForeign = function (columns, indexName) {
});
};
// Compile a drop primary key command.
TableCompiler_SQLite3.prototype.dropPrimary = function (constraintName) {
const compiler = this;
this.pushQuery({
sql: `PRAGMA table_info(${this.tableName()})`,
output(pragma) {
return compiler.client
.ddl(compiler, pragma, this.connection)
.dropPrimary(constraintName);
},
});
};
TableCompiler_SQLite3.prototype.dropIndex = function (columns, indexName) {
indexName = indexName
? this.formatter.wrap(indexName)
@ -93,11 +107,66 @@ TableCompiler_SQLite3.prototype.index = function (columns, indexName) {
);
};
TableCompiler_SQLite3.prototype.primary = TableCompiler_SQLite3.prototype.foreign = function () {
/**
* Add a primary key to an existing table.
*
* @NOTE The `createQuery` method above handles table creation. Don't do anything regarding table
* creation in this method
*
* @param {string | string[]} columns - Column name(s) to assign as primary keys
* @param {string} [constraintName] - Custom name for the PK constraint
*/
TableCompiler_SQLite3.prototype.primary = function (columns, constraintName) {
const compiler = this;
columns = this.formatter.columnize(columns);
columns = Array.isArray(columns) ? columns : [columns];
if (this.method !== 'create' && this.method !== 'createIfNot') {
this.client.logger.warn(
'SQLite3 Foreign & Primary keys may only be added on create'
);
this.pushQuery({
sql: `PRAGMA table_info(${this.tableName()})`,
output(pragma) {
return compiler.client
.ddl(compiler, pragma, this.connection)
.primary(columns, constraintName);
},
});
}
};
/**
* Add a foreign key constraint to an existing table
*
* @NOTE The `createQuery` method above handles foreign key constraints on table creation. Don't do
* anything regarding table creation in this method
*
* @param {object} foreignInfo - Information about the current column foreign setup
* @param {string | string[]} [foreignInfo.column] - Column in the current constraint
* @param {string | undefined} foreignInfo.keyName - Name of the foreign key constraint
* @param {string} foreignInfo.references - What column it references in the other table
* @param {string} foreignInfo.inTable - What table is referenced in this constraint
* @param {string} [foreignInfo.onUpdate] - What to do on updates
* @param {string} [foreignInfo.onDelete] - What to do on deletions
*/
TableCompiler_SQLite3.prototype.foreign = function (foreignInfo) {
const compiler = this;
if (this.method !== 'create' && this.method !== 'createIfNot') {
foreignInfo.column = this.formatter.columnize(foreignInfo.column);
foreignInfo.column = Array.isArray(foreignInfo.column)
? foreignInfo.column
: [foreignInfo.column];
foreignInfo.inTable = this.formatter.columnize(foreignInfo.inTable);
foreignInfo.references = this.formatter.columnize(foreignInfo.references);
this.pushQuery({
sql: `PRAGMA table_info(${this.tableName()})`,
output(pragma) {
return compiler.client
.ddl(compiler, pragma, this.connection)
.foreign(foreignInfo);
},
});
}
};

View File

@ -8,6 +8,7 @@ describe('Util Tests', function () {
require('./unit/util/fs');
require('./unit/util/nanoid');
require('./unit/util/save-async-stack');
require('./unit/util/comma-no-paren-regex');
});
describe('Query Building Tests', function () {

View File

@ -48,7 +48,7 @@ module.exports = (knex) => {
`insert into \`foreign_keys_table_one\` (\`fkey_three\`, \`fkey_two\`) values (99, 9999) - SQLITE_CONSTRAINT: FOREIGN KEY constraint failed`
);
}
if (knex.client.driverName === 'postgres') {
if (knex.client.driverName === 'pg') {
expect(err.message).to.equal(
`insert into "foreign_keys_table_one" ("fkey_three", "fkey_two") values ($1, $2) - insert or update on table "foreign_keys_table_one" violates foreign key constraint "foreign_keys_table_one_fkey_two_foreign"`
);

View File

@ -1,7 +1,7 @@
const { expect } = require('chai');
const { getAllDbs, getKnexForDb } = require('../util/knex-instance-provider');
describe.skip('Schema', () => {
describe('Schema', () => {
describe('Foreign keys', () => {
getAllDbs().forEach((db) => {
describe(db, () => {
@ -63,9 +63,9 @@ describe.skip('Schema', () => {
`insert into \`foreign_keys_table_one\` (\`fkey_three\`, \`fkey_two\`) values (99, 9999) - SQLITE_CONSTRAINT: FOREIGN KEY constraint failed`
);
}
if (knex.client.driverName === 'postgres') {
if (knex.client.driverName === 'pg') {
expect(err.message).to.equal(
`insert into "foreign_keys_table_one" ("fkey_three", "fkey_two") values ($1, $2) - insert or update on table "foreign_keys_table_one" violates foreign key constraint "foreign_keys_table_one_fkey_two_foreign"`
`insert into "foreign_keys_table_one" ("fkey_three", "fkey_two") values ($1, $2) - insert or update on table "foreign_keys_table_one" violates foreign key constraint "fk_fkey_threeee"`
);
}
expect(err.message).to.include('constraint');

View File

@ -0,0 +1,149 @@
const { expect } = require('chai');
const { getAllDbs, getKnexForDb } = require('../util/knex-instance-provider');
describe('Schema', () => {
describe('Primary keys', () => {
getAllDbs().forEach((db) => {
describe(db, () => {
let knex;
before(function () {
knex = getKnexForDb(db);
if (knex.client.driverName === 'mssql') {
return this.skip();
}
});
after(() => {
return knex.destroy();
});
beforeEach(async () => {
await knex.schema.createTable('primary_table', (table) => {
table.integer('id_one');
table.integer('id_two');
table.integer('id_three');
});
});
afterEach(async () => {
await knex.schema.dropTable('primary_table');
});
describe('createPrimaryKey', () => {
it('creates a new primary key', async () => {
await knex.schema.alterTable('primary_table', (table) => {
table.integer('id_four').primary();
});
await knex('primary_table').insert({ id_four: 1 });
try {
await knex('primary_table').insert({ id_four: 1 });
throw new Error(`Shouldn't reach this`);
} catch (err) {
if (knex.client.driverName === 'sqlite3') {
expect(err.message).to.equal(
'insert into `primary_table` (`id_four`) values (1) - SQLITE_CONSTRAINT: UNIQUE constraint failed: primary_table.id_four'
);
}
if (knex.client.driverName === 'pg') {
expect(err.message).to.equal(
'insert into "primary_table" ("id_four") values ($1) - duplicate key value violates unique constraint "primary_table_pkey"'
);
}
}
});
it('creates a primary key with a custom constraint name', async () => {
await knex.schema.alterTable('primary_table', (table) => {
table.integer('id_four').primary('my_custom_constraint_name');
});
await knex('primary_table').insert({ id_four: 1 });
try {
await knex('primary_table').insert({ id_four: 1 });
throw new Error(`Shouldn't reach this`);
} catch (err) {
if (knex.client.driverName === 'sqlite3') {
expect(err.message).to.equal(
'insert into `primary_table` (`id_four`) values (1) - SQLITE_CONSTRAINT: UNIQUE constraint failed: primary_table.id_four'
);
}
if (knex.client.driverName === 'pg') {
expect(err.message).to.equal(
'insert into "primary_table" ("id_four") values ($1) - duplicate key value violates unique constraint "my_custom_constraint_name"'
);
}
}
await knex.schema.alterTable('primary_table', (table) => {
table.dropPrimary('my_custom_constraint_name');
});
await knex('primary_table').insert({ id_four: 1 });
});
it('creates a compound primary key', async () => {
await knex.schema.alterTable('primary_table', (table) => {
table.primary(['id_two', 'id_three']);
});
await knex('primary_table').insert({ id_two: 1, id_three: 1 });
await knex('primary_table').insert({ id_two: 2, id_three: 1 });
await knex('primary_table').insert({ id_two: 1, id_three: 2 });
try {
await knex('primary_table').insert({ id_two: 1, id_three: 1 });
} catch (err) {
if (knex.client.driverName === 'sqlite3') {
expect(err.message).to.equal(
'insert into `primary_table` (`id_three`, `id_two`) values (1, 1) - SQLITE_CONSTRAINT: UNIQUE constraint failed: primary_table.id_two, primary_table.id_three'
);
}
if (knex.client.driverName === 'pg') {
expect(err.message).to.equal(
'insert into "primary_table" ("id_three", "id_two") values ($1, $2) - duplicate key value violates unique constraint "primary_table_pkey"'
);
}
}
});
it('creates a compound primary key with a custom constraint name', async () => {
await knex.schema.alterTable('primary_table', (table) => {
table.primary(
['id_two', 'id_three'],
'my_custom_constraint_name'
);
});
await knex('primary_table').insert({ id_two: 1, id_three: 1 });
await knex('primary_table').insert({ id_two: 2, id_three: 1 });
await knex('primary_table').insert({ id_two: 1, id_three: 2 });
try {
await knex('primary_table').insert({ id_two: 1, id_three: 1 });
} catch (err) {
if (knex.client.driverName === 'sqlite3') {
expect(err.message).to.equal(
'insert into `primary_table` (`id_three`, `id_two`) values (1, 1) - SQLITE_CONSTRAINT: UNIQUE constraint failed: primary_table.id_two, primary_table.id_three'
);
}
if (knex.client.driverName === 'pg') {
expect(err.message).to.equal(
'insert into "primary_table" ("id_three", "id_two") values ($1, $2) - duplicate key value violates unique constraint "my_custom_constraint_name"'
);
}
}
await knex.schema.alterTable('primary_table', (table) => {
table.dropPrimary('my_custom_constraint_name');
});
await knex('primary_table').insert({ id_two: 1, id_three: 1 });
});
});
});
});
});
});

View File

@ -0,0 +1,18 @@
const { COMMA_NO_PAREN_REGEX } = require('../../../lib/constants');
const { expect } = require('chai');
describe('COMMA_NO_PAREN Regex', () => {
it('Splits string on commas, excluding those wrapped in parentheses', () => {
const source = `id integer, external_id integer, name string, body text, PRIMARY KEY (id, external_id)`;
const result = source.split(COMMA_NO_PAREN_REGEX);
expect(result).to.eql([
'id integer',
'external_id integer',
'name string',
'body text',
'PRIMARY KEY (id, external_id)',
]);
});
});