Fix identifier formatting for migrations when used with Oracledb

lib/dialects/oracle/schema/internal/incrementUtils.js

@@ -6,7 +6,10 @@ function createAutoIncrementTriggerAndSequence(columnCompiler) {
 
   // TODO Add warning that sequence etc is created
   columnCompiler.pushAdditional(function () {
-    const tableName = this.tableCompiler.tableNameRaw;
+    // Get formatted table name without quotes
+    const tableName = this.tableCompiler.formatter
+      .wrap(this.tableCompiler.tableNameRaw)
+      .slice(1, -1);
     const schemaName = this.tableCompiler.schemaNameRaw;
     const createTriggerSQL = trigger.createAutoIncrementTrigger(
       this.client.logger,

lib/dialects/oracle/schema/oracle-compiler.js

@@ -11,11 +11,14 @@ class SchemaCompiler_Oracle extends SchemaCompiler {
 
   // Rename a table on the schema.
   renameTable(tableName, to) {
+    // Format table names and remove quotes
+    const fromTableName = this.formatter.wrap(tableName).slice(1, -1);
+    const toTableName = this.formatter.wrap(to).slice(1, -1);
     const trigger = new Trigger(this.client.version);
     const renameTable = trigger.renameTableAndAutoIncrementTrigger(
       this.client.logger,
-      tableName,
-      to
+      fromTableName,
+      toTableName
     );
     this.pushQuery(renameTable);
   }
@@ -67,7 +70,12 @@ class SchemaCompiler_Oracle extends SchemaCompiler {
       'seq',
       tableName
     );
-    this.dropSequenceIfExists(sequenceName);
+    // Auto-generated sequence and trigger names are not processed by custom wrapIdentifier.
+    // Delete will fail if this.formatter.wrap() is called when knexfile specifies wrapIdentifier.
+    const prefix = this.schema ? `"${this.schema}".` : '';
+    this.pushQuery(
+      utils.wrapSqlWithCatch(`drop sequence ${prefix}"${sequenceName}"`, -2289)
+    );
   }
 
   dropTable(tableName) {

lib/dialects/oracle/schema/oracle-tablecompiler.js

@@ -39,9 +39,12 @@ class TableCompiler_Oracle extends TableCompiler {
   renameColumn(from, to) {
     // Remove quotes around tableName
     const tableName = this.tableName().slice(1, -1);
+    // Format column names and remove quotes
+    const colFrom = this.formatter.wrap(from).slice(1, -1);
+    const colTo = this.formatter.wrap(to).slice(1, -1);
     const trigger = new Trigger(this.client.version);
     return this.pushQuery(
-      trigger.renameColumnTrigger(this.client.logger, tableName, from, to)
+      trigger.renameColumnTrigger(this.client.logger, tableName, colFrom, colTo)
     );
   }
 

test/unit/schema-builder/oracle.js

@@ -1011,10 +1011,11 @@ describe('Oracle SchemaBuilder', function () {
         })
         .toSQL();
 
-      expect(spy.callCount).to.equal(3);
+      expect(spy.callCount).to.equal(4);
       expect(spy.firstCall.args).to.deep.equal(['id', 'table context']);
-      expect(spy.secondCall.args).to.deep.equal(['email', 'table context']);
-      expect(spy.thirdCall.args).to.deep.equal(['users', 'table context']);
+      expect(spy.secondCall.args).to.deep.equal(['users', 'table context']);
+      expect(spy.thirdCall.args).to.deep.equal(['email', 'table context']);
+      expect(spy.lastCall.args).to.deep.equal(['users', 'table context']);
     });
 
     it('TableCompiler passes queryContext to wrapIdentifier', function () {
@@ -1026,10 +1027,11 @@ describe('Oracle SchemaBuilder', function () {
         })
         .toSQL();
 
-      expect(spy.callCount).to.equal(3);
+      expect(spy.callCount).to.equal(4);
       expect(spy.firstCall.args).to.deep.equal(['id', 'id context']);
-      expect(spy.secondCall.args).to.deep.equal(['email', 'email context']);
-      expect(spy.thirdCall.args).to.deep.equal(['users', undefined]);
+      expect(spy.secondCall.args).to.deep.equal(['users', undefined]);
+      expect(spy.thirdCall.args).to.deep.equal(['email', 'email context']);
+      expect(spy.lastCall.args).to.deep.equal(['users', undefined]);
     });
 
     it('TableCompiler allows overwriting queryContext from SchemaCompiler', function () {
@@ -1043,10 +1045,11 @@ describe('Oracle SchemaBuilder', function () {
         })
         .toSQL();
 
-      expect(spy.callCount).to.equal(3);
+      expect(spy.callCount).to.equal(4);
       expect(spy.firstCall.args).to.deep.equal(['id', 'table context']);
-      expect(spy.secondCall.args).to.deep.equal(['email', 'table context']);
-      expect(spy.thirdCall.args).to.deep.equal(['users', 'table context']);
+      expect(spy.secondCall.args).to.deep.equal(['users', 'table context']);
+      expect(spy.thirdCall.args).to.deep.equal(['email', 'table context']);
+      expect(spy.lastCall.args).to.deep.equal(['users', 'table context']);
     });
 
     it('ColumnCompiler allows overwriting queryContext from TableCompiler', function () {
@@ -1060,10 +1063,11 @@ describe('Oracle SchemaBuilder', function () {
         })
         .toSQL();
 
-      expect(spy.callCount).to.equal(3);
+      expect(spy.callCount).to.equal(4);
       expect(spy.firstCall.args).to.deep.equal(['id', 'id context']);
-      expect(spy.secondCall.args).to.deep.equal(['email', 'email context']);
-      expect(spy.thirdCall.args).to.deep.equal(['users', 'table context']);
+      expect(spy.secondCall.args).to.deep.equal(['users', 'table context']);
+      expect(spy.thirdCall.args).to.deep.equal(['email', 'email context']);
+      expect(spy.lastCall.args).to.deep.equal(['users', 'table context']);
     });
   });
 

test/unit/schema-builder/oracledb.js

@@ -1192,6 +1192,80 @@ describe('OracleDb SchemaBuilder', function () {
     );
   });
 
+  describe('wrapIdentifier', function () {
+    let spy;
+    let originalWrapIdentifier;
+
+    before(function () {
+      spy = sinon.spy();
+      originalWrapIdentifier = client.config.wrapIdentifier;
+      client.config.wrapIdentifier = function (value, wrap) {
+        spy(value);
+        return wrap(value);
+      };
+    });
+
+    beforeEach(function () {
+      spy.resetHistory();
+    });
+
+    after(function () {
+      client.config.wrapIdentifier = originalWrapIdentifier;
+    });
+
+    it('TableCompiler createQuery should format table name and ColumnCompiler should not format increments trigger and sequence names', function () {
+      client
+        .schemaBuilder()
+        .createTable('users', function (table) {
+          table.increments('id');
+          table.string('email');
+        })
+        .toSQL();
+
+      expect(spy.callCount).to.equal(4);
+      expect(spy.firstCall.args).to.deep.equal(['id']);
+      expect(spy.secondCall.args).to.deep.equal(['users']);
+      expect(spy.thirdCall.args).to.deep.equal(['email']);
+      expect(spy.lastCall.args).to.deep.equal(['users']);
+    });
+
+    it('TableCompiler renameColumn should format provided column names while not formatting increments trigger and sequence names', function () {
+      client
+        .schemaBuilder()
+        .alterTable('users', function (table) {
+          table.renameColumn('increments1', 'increments2');
+        })
+        .toSQL();
+
+      expect(spy.callCount).to.equal(3);
+      expect(spy.firstCall.args).to.deep.equal(['users']);
+      expect(spy.secondCall.args).to.deep.equal(['increments1']);
+      expect(spy.thirdCall.args).to.deep.equal(['increments2']);
+    });
+
+    it('SchemaCompiler renameTable should format provided table names while not formatting increments trigger and sequence names', function () {
+      client.schemaBuilder().renameTable('old', 'new').toSQL();
+
+      expect(spy.callCount).to.equal(2);
+      expect(spy.firstCall.args).to.deep.equal(['old']);
+      expect(spy.secondCall.args).to.deep.equal(['new']);
+    });
+
+    it('SchemaCompiler dropTable should not reformat increments sequence and trigger names', function () {
+      client.schemaBuilder().dropTable('users').toSQL();
+
+      expect(spy.callCount).to.equal(1);
+      expect(spy.firstCall.args).to.deep.equal(['users']);
+    });
+
+    it('SchemaCompiler dropTableIfExists should not reformat increments sequence and trigger names', function () {
+      client.schemaBuilder().dropTableIfExists('users').toSQL();
+
+      expect(spy.callCount).to.equal(1);
+      expect(spy.firstCall.args).to.deep.equal(['users']);
+    });
+  });
+
   describe('queryContext', function () {
     let spy;
     let originalWrapIdentifier;
@@ -1223,10 +1297,11 @@ describe('OracleDb SchemaBuilder', function () {
         })
         .toSQL();
 
-      expect(spy.callCount).to.equal(3);
+      expect(spy.callCount).to.equal(4);
       expect(spy.firstCall.args).to.deep.equal(['id', 'table context']);
-      expect(spy.secondCall.args).to.deep.equal(['email', 'table context']);
-      expect(spy.thirdCall.args).to.deep.equal(['users', 'table context']);
+      expect(spy.secondCall.args).to.deep.equal(['users', 'table context']);
+      expect(spy.thirdCall.args).to.deep.equal(['email', 'table context']);
+      expect(spy.lastCall.args).to.deep.equal(['users', 'table context']);
     });
 
     it('TableCompiler passes queryContext to wrapIdentifier', function () {
@@ -1238,10 +1313,11 @@ describe('OracleDb SchemaBuilder', function () {
         })
         .toSQL();
 
-      expect(spy.callCount).to.equal(3);
+      expect(spy.callCount).to.equal(4);
       expect(spy.firstCall.args).to.deep.equal(['id', 'id context']);
-      expect(spy.secondCall.args).to.deep.equal(['email', 'email context']);
-      expect(spy.thirdCall.args).to.deep.equal(['users', undefined]);
+      expect(spy.secondCall.args).to.deep.equal(['users', undefined]);
+      expect(spy.thirdCall.args).to.deep.equal(['email', 'email context']);
+      expect(spy.lastCall.args).to.deep.equal(['users', undefined]);
     });
 
     it('TableCompiler allows overwriting queryContext from SchemaCompiler', function () {
@@ -1255,10 +1331,11 @@ describe('OracleDb SchemaBuilder', function () {
         })
         .toSQL();
 
-      expect(spy.callCount).to.equal(3);
+      expect(spy.callCount).to.equal(4);
       expect(spy.firstCall.args).to.deep.equal(['id', 'table context']);
-      expect(spy.secondCall.args).to.deep.equal(['email', 'table context']);
-      expect(spy.thirdCall.args).to.deep.equal(['users', 'table context']);
+      expect(spy.secondCall.args).to.deep.equal(['users', 'table context']);
+      expect(spy.thirdCall.args).to.deep.equal(['email', 'table context']);
+      expect(spy.lastCall.args).to.deep.equal(['users', 'table context']);
     });
 
     it('ColumnCompiler allows overwriting queryContext from TableCompiler', function () {
@@ -1272,10 +1349,11 @@ describe('OracleDb SchemaBuilder', function () {
         })
         .toSQL();
 
-      expect(spy.callCount).to.equal(3);
+      expect(spy.callCount).to.equal(4);
       expect(spy.firstCall.args).to.deep.equal(['id', 'id context']);
-      expect(spy.secondCall.args).to.deep.equal(['email', 'email context']);
-      expect(spy.thirdCall.args).to.deep.equal(['users', 'table context']);
+      expect(spy.secondCall.args).to.deep.equal(['users', 'table context']);
+      expect(spy.thirdCall.args).to.deep.equal(['email', 'email context']);
+      expect(spy.lastCall.args).to.deep.equal(['users', 'table context']);
     });
   });