diff --git a/lib/dialects/sqlite3/schema/ddl.js b/lib/dialects/sqlite3/schema/ddl.js index 34e722eee8..67d52ea76d 100644 --- a/lib/dialects/sqlite3/schema/ddl.js +++ b/lib/dialects/sqlite3/schema/ddl.js @@ -14,6 +14,9 @@ const { omit, invert, fromPairs, + some, + negate, + isEmpty } = require('lodash'); // So altering the schema in SQLite3 is a major pain. @@ -40,8 +43,8 @@ assign(SQLite3_DDL.prototype, { getColumn: async function(column) { const currentCol = find(this.pragma, (col) => { return ( - this.client.wrapIdentifier(col.name) === - this.client.wrapIdentifier(column) + this.client.wrapIdentifier(col.name).toLowerCase() === + this.client.wrapIdentifier(column).toLowerCase() ); }); if (!currentCol) @@ -130,7 +133,7 @@ assign(SQLite3_DDL.prototype, { _doReplace(sql, from, to) { const oneLineSql = sql.replace(/\s+/g, ' '); - const matched = oneLineSql.match(/^CREATE TABLE (\S+) \((.*)\)/); + const matched = oneLineSql.match(/^CREATE TABLE\s+(\S+)\s*\((.*)\)/); const tableName = matched[1]; const defs = matched[2]; @@ -167,27 +170,34 @@ assign(SQLite3_DDL.prototype, { } args.push(defs.slice(ptr, i)); - const fromIdentifier = from.replace(/[`"]/g, ''); + const fromIdentifier = from.replace(/[`"'[\]]/g, ''); args = args.map(function(item) { let split = item.trim().split(' '); - let normalizedFrom = from; - - // Backwards compatible for double quoted sqlite databases - // The "from" and "to" field use backsticks, because this is the default notation for - // SQlite3 since Knex 0.14. - // e.g. from: `about` + // SQLite supports all quoting mechanisms prevalent in all major dialects of SQL + // and preserves the original quoting in sqlite_master. + // + // Also, identifiers are never case sensitive, not even when quoted. // - // We have to replace the from+to field with double slashes in case you created your SQlite3 - // database with Knex < 0.14. - if (item.match(`"${fromIdentifier}"`)) { - normalizedFrom = `"${fromIdentifier}"`; - } else if (item.match(`\`${fromIdentifier}\``)) { - normalizedFrom = `\`${fromIdentifier}\``; + // Ref: https://www.sqlite.org/lang_keywords.html + const fromMatchCandidates = [ + new RegExp(`\`${fromIdentifier}\``, 'i'), + new RegExp(`"${fromIdentifier}"`, 'i'), + new RegExp(`'${fromIdentifier}'`, 'i'), + new RegExp(`\\[${fromIdentifier}\\]`, 'i') + ]; + if (fromIdentifier.match(/^\S+$/)) { + fromMatchCandidates.push(new RegExp(`\\b${fromIdentifier}\\b`, 'i')); } - if (split[0] === normalizedFrom) { + const doesMatchFromIdentifier = (target) => + some(fromMatchCandidates, (c) => target.match(c)); + + const replaceFromIdentifier = (target) => + fromMatchCandidates.reduce((result, candidate) => result.replace(candidate, to), target); + + if (doesMatchFromIdentifier(split[0])) { // column definition if (to) { split[0] = to; @@ -203,9 +213,11 @@ assign(SQLite3_DDL.prototype, { // columns from this table listed between (); replace // one if it matches if (/primary|unique/i.test(split[idx])) { - return item.replace(/\(.*\)/, (columns) => - columns.replace(normalizedFrom, to) - ); + const ret = item.replace(/\(.*\)/, replaceFromIdentifier); + // If any member columns are dropped then uniqueness/pk constraint + // can not be retained + if (ret !== item && isEmpty(to)) return ''; + return ret; } // foreign keys have one or more columns from this table @@ -217,18 +229,37 @@ assign(SQLite3_DDL.prototype, { split = item.split(/ references /i); // the quoted column names save us from having to do anything // other than a straight replace here - split[0] = split[0].replace(normalizedFrom, to); + const replacedKeySpec = replaceFromIdentifier(split[0]); + + if (split[0] !== replacedKeySpec) { + // If we are removing one or more columns of a foreign + // key, then we should not retain the key at all + if (isEmpty(to)) return ''; + else split[0] = replacedKeySpec; + } if (split[1].slice(0, tableName.length) === tableName) { - split[1] = split[1].replace(/\(.*\)/, (columns) => - columns.replace(normalizedFrom, to) - ); + // self-referential foreign key + const replacedKeyTargetSpec = split[1].replace(/\(.*\)/, replaceFromIdentifier); + if (split[1] !== replacedKeyTargetSpec) { + // If we are removing one or more columns of a foreign + // key, then we should not retain the key at all + if (isEmpty(to)) return ''; + else split[1] = replacedKeyTargetSpec; + } } return split.join(' references '); } return item; }); + + args = args.filter(negate(isEmpty)); + + if (args.length === 0) { + throw new Error('Unable to drop last column from table'); + } + return oneLineSql .replace(/\(.*\)/, () => `(${args.join(', ')})`) .replace(/,\s*([,)])/, '$1'); @@ -248,6 +279,7 @@ assign(SQLite3_DDL.prototype, { if (sql === newSql) { throw new Error('Unable to find the column to change'); } + const { from: mappedFrom, to: mappedTo } = invert( this.client.postProcessResponse( invert({ diff --git a/test/integration/schema/index.js b/test/integration/schema/index.js index c53117ee92..2c2943cc18 100644 --- a/test/integration/schema/index.js +++ b/test/integration/schema/index.js @@ -1382,6 +1382,81 @@ module.exports = function(knex) { }); } }); + + context('when table is created using raw create table', function () { + beforeEach(async function() { + await knex.schema.raw(`create table TEST( + "i0" integer, + 'i1' integer, + [i2] integer, + \`i3\` integer, + i4 integer, + I5 integer, + unique(i4, i5), + constraint i0 primary key([i3], "i4"), + unique([i2]), + foreign key (i1) references bar ("i3") + )`); + }); + + afterEach(function() { + return knex.schema.dropTable('TEST'); + }); + + const getCreateTableExpr = async () => ( + await knex.schema.raw('select name, sql from sqlite_master where type = "table" and name = "TEST"') + )[0].sql; + + const dropCol = (colName) => + knex.schema.alterTable('TEST', function(tbl) { + return tbl.dropColumn(colName); + }); + + const hasCol = (colName) => + knex.schema.hasColumn('TEST', colName); + + it('drops the column', async function() { + await dropCol('i0'); + expect(await hasCol('i0')).to.equal(false); + // Constraint i0 should be unaffected: + expect(await getCreateTableExpr()).to.equal( + "CREATE TABLE TEST('i1' integer, [i2] integer, `i3` integer, i4 " + + 'integer, I5 integer, unique(i4, i5), constraint i0 primary ' + + 'key([i3], "i4"), unique([i2]), foreign key (i1) references bar ' + + '("i3") )' + ); + await dropCol('i1'); + expect(await hasCol('i1')).to.equal(false); + // Foreign key on i1 should also be dropped: + expect(await getCreateTableExpr()).to.equal( + 'CREATE TABLE TEST([i2] integer, `i3` integer, i4 integer, I5 integer, ' + + 'unique(i4, i5), constraint i0 primary key([i3], "i4"), unique([i2]))' + ); + await dropCol('i2'); + expect(await hasCol('i2')).to.equal(false); + expect(await getCreateTableExpr()).to.equal( + 'CREATE TABLE TEST(`i3` integer, i4 integer, I5 integer, ' + + 'unique(i4, i5), constraint i0 primary key([i3], "i4"))' + ); + await dropCol('i3'); + expect(await hasCol('i3')).to.equal(false); + expect(await getCreateTableExpr()).to.equal( + 'CREATE TABLE TEST(i4 integer, I5 integer, unique(i4, i5))' + ); + await dropCol('i4'); + expect(await hasCol('i4')).to.equal(false); + expect(await getCreateTableExpr()).to.equal( + 'CREATE TABLE TEST(I5 integer)' + ); + let lastColDeletionError; + await knex.schema.alterTable('TEST', function(tbl) { + return tbl.dropColumn('i5'); + }).catch(e => { + lastColDeletionError = e; + }) + expect(lastColDeletionError.message).to.eql('Unable to drop last column from table'); + }); + }); } });