Fix handling of multiline SQL in SQLite3 schema (#3411)

lib/dialects/sqlite3/schema/ddl.js

@@ -129,7 +129,8 @@ assign(SQLite3_DDL.prototype, {
   },
 
   _doReplace(sql, from, to) {
-    const matched = sql.match(/^CREATE TABLE (\S+) \((.*)\)/);
+    const oneLineSql = sql.replace(/\s+/g, ' ');
+    const matched = oneLineSql.match(/^CREATE TABLE (\S+) \((.*)\)/);
 
     const tableName = matched[1];
     const defs = matched[2];
@@ -166,23 +167,27 @@ assign(SQLite3_DDL.prototype, {
     }
     args.push(defs.slice(ptr, i));
 
-    // Backwards compatible for double quoted sqlite databases
-    // Detect CREATE TABLE "accounts" ("id"...)
-    // The "from" and "to" field use backsticks, because this is the default notation for
-    // SQlite3 since Knex 0.14.
-    // e.g. from: `about`
-    //
-    // We have to replace the from+to field with double slashes in case you created your SQlite3
-    // database with Knex < 0.14.
-    if (sql.match(/CREATE\sTABLE\s".*"\s\("/)) {
-      from = from.replace(/[`]/g, '"');
-      to = to.replace(/[`]/g, '"');
-    }
+    const fromIdentifier = from.replace(/[`"]/g, '');
 
     args = args.map(function(item) {
-      let split = item.split(' ');
+      let split = item.trim().split(' ');
 
-      if (split[0] === from) {
+      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`
+      //
+      // 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}\``;
+      }
+
+      if (split[0] === normalizedFrom) {
         // column definition
         if (to) {
           split[0] = to;
@@ -198,7 +203,9 @@ 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(from, to));
+        return item.replace(/\(.*\)/, (columns) =>
+          columns.replace(normalizedFrom, to)
+        );
       }
 
       // foreign keys have one or more columns from this table
@@ -210,11 +217,11 @@ 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(from, to);
+        split[0] = split[0].replace(normalizedFrom, to);
 
         if (split[1].slice(0, tableName.length) === tableName) {
           split[1] = split[1].replace(/\(.*\)/, (columns) =>
-            columns.replace(from, to)
+            columns.replace(normalizedFrom, to)
           );
         }
         return split.join(' references ');
@@ -222,7 +229,7 @@ assign(SQLite3_DDL.prototype, {
 
       return item;
     });
-    return sql
+    return oneLineSql
       .replace(/\(.*\)/, () => `(${args.join(', ')})`)
       .replace(/,\s*([,)])/, '$1');
   },

test/integration/index.js

@@ -5,18 +5,22 @@ const logger = require('./logger');
 const config = require('../knexfile');
 const fs = require('fs');
 
-const Bluebird = require('bluebird');
-
 Object.keys(config).forEach((dialectName) => {
   return require('./suite')(logger(knex(config[dialectName])));
 });
 
-after(function(done) {
+before(function() {
   if (config.sqlite3 && config.sqlite3.connection.filename !== ':memory:') {
-    fs.unlink(config.sqlite3.connection.filename, function() {
-      done();
-    });
-  } else {
-    done();
+    fs.copyFileSync(
+      __dirname + '/../multilineCreateMasterSample.sqlite3',
+      __dirname + '/../multilineCreateMaster.sqlite3'
+    );
+  }
+});
+
+after(function() {
+  if (config.sqlite3 && config.sqlite3.connection.filename !== ':memory:') {
+    fs.unlinkSync(config.sqlite3.connection.filename);
+    fs.unlinkSync(__dirname + '/../multilineCreateMaster.sqlite3');
   }
 });

test/integration/migrate/index.js

@@ -6,6 +6,10 @@ const fs = require('fs');
 const path = require('path');
 const rimraf = require('rimraf');
 const Bluebird = require('bluebird');
+const knexLib = require('../../../knex');
+const logger = require('../logger');
+const config = require('../../knexfile');
+const _ = require('lodash');
 const testMemoryMigrations = require('./memory-migrations');
 
 module.exports = function(knex) {
@@ -72,6 +76,25 @@ module.exports = function(knex) {
       });
     }
 
+    if (knex.client.driverName === 'sqlite3') {
+      it('should not fail rename-and-drop-column with multiline sql from legacy db', async () => {
+        const knexConfig = _.extend({}, config.sqlite3, {
+          connection: {
+            filename: __dirname + '/../../multilineCreateMaster.sqlite3',
+          },
+          migrations: {
+            directory:
+              'test/integration/migrate/rename-and-drop-column-with-multiline-sql-from-legacy-db',
+          },
+        });
+
+        const db = logger(knexLib(knexConfig));
+
+        await db.migrate.latest();
+        await db.migrate.rollback();
+      });
+    }
+
     it('should not fail drop-and-recreate-column operation when using async/await', () => {
       return knex.migrate
         .latest({

test/integration/migrate/rename-and-drop-column-with-multiline-sql-from-legacy-db/01_rename-column.js

@@ -0,0 +1,11 @@
+exports.up = function(knex) {
+  return knex.schema.table('TestTableCreatedWithDBBrowser', function(table) {
+    table.renameColumn('id', 'testId');
+  });
+};
+
+exports.down = function(knex) {
+  return knex.schema.table('TestTableCreatedWithDBBrowser', function(table) {
+    table.renameColumn('testId', 'id');
+  });
+};

test/integration/migrate/rename-and-drop-column-with-multiline-sql-from-legacy-db/02_drop_column.js

@@ -0,0 +1,11 @@
+exports.up = function(knex) {
+  return knex.schema.table('TestTableCreatedWithDBBrowser', function(table) {
+    table.dropColumn('description');
+  });
+};
+
+exports.down = function(knex) {
+  return knex.schema.table('TestTableCreatedWithDBBrowser', function(table) {
+    table.text('description');
+  });
+};

test/unit/dialects/sqlite3.js

@@ -16,7 +16,7 @@ it('[backwards compatible] can rename column with double quotes', function() {
 
   const newSql = ddl._doReplace(sql, '`about`', '`about_me`');
   newSql.should.eql(
-    'CREATE TABLE "accounts" ("id" varchar(24) not null primary key, "about_me" varchar(24))'
+    'CREATE TABLE "accounts" ("id" varchar(24) not null primary key, `about_me` varchar(24))'
   );
 });
 
@@ -48,7 +48,7 @@ it('[backwards compatible] can rename column with double quotes', function() {
 
   const newSql = ddl._doReplace(sql, '"about"', '`about_me`');
   newSql.should.eql(
-    'CREATE TABLE "accounts" ("id" varchar(24) not null primary key, "about_me" varchar(24))'
+    'CREATE TABLE "accounts" ("id" varchar(24) not null primary key, `about_me` varchar(24))'
   );
 });
 
@@ -67,3 +67,35 @@ it('can rename column with back sticks', function() {
     'CREATE TABLE `accounts` (`id` varchar(24) not null primary key, `about_me` varchar(24))'
   );
 });
+
+it('can rename column with multiline and tabulated sql statement', function() {
+  const client = sinon.stub();
+  const tableCompiler = sinon.stub();
+  const pragma = sinon.stub();
+  const connection = sinon.stub();
+  const ddl = new DDL(client, tableCompiler, pragma, connection);
+
+  const sql =
+    'CREATE TABLE `accounts` (\n\n`id`\tvarchar(24) not null primary key,\r\n`about`\t \t\t \tvarchar(24)\n\n)';
+
+  const newSql = ddl._doReplace(sql, '`about`', '`about_me`');
+  newSql.should.eql(
+    'CREATE TABLE `accounts` (`id` varchar(24) not null primary key, `about_me` varchar(24))'
+  );
+});
+
+it('can drop column with multiline and tabulated sql statement', function() {
+  const client = sinon.stub();
+  const tableCompiler = sinon.stub();
+  const pragma = sinon.stub();
+  const connection = sinon.stub();
+  const ddl = new DDL(client, tableCompiler, pragma, connection);
+
+  const sql =
+    'CREATE TABLE `accounts` (\n\n`id`\tvarchar(24) not null primary key,\r\n`about`\t \tvarchar(24)\n)';
+
+  const newSql = ddl._doReplace(sql, '`about`', '');
+  newSql.should.eql(
+    'CREATE TABLE `accounts` (`id` varchar(24) not null primary key)'
+  );
+});