fix wrapIdentifier not being called in postgres alter column (#2612)

* fix wrapIdentifier not being called in postgres alter column * add test for wrapIdentifier call in postgres alter column * add comment regarding issue * add issue & PR #'s in comment
knex · May 30, 2018 · 3e95405 · 3e95405
1 parent 4d4e4ca
commit 3e95405
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 6 deletions.
diff --git a/src/dialects/postgres/schema/tablecompiler.js b/src/dialects/postgres/schema/tablecompiler.js
@@ -46,35 +46,39 @@ TableCompiler_PG.prototype.addColumns = function(columns, prefix, colCompilers)
     // alter columns
     for (const col  of colCompilers) {
       const quotedTableName = this.tableName();
-      const colName = col.getColumnName();
       const type = col.getColumnType();
+      // We'd prefer to call this.formatter.wrapAsIdentifier here instead, however the context passed to
+      // `this` instance is not that of the column, but of the table. Thus, we unfortunately have to call
+      // `wrapIdentifier` here as well (it is already called once on the initial column operation) to give
+      // our `alter` operation the correct `queryContext`. Refer to issue #2606 and PR #2612.
+      const colName = this.client.wrapIdentifier(col.getColumnName(), col.columnBuilder.queryContext());
 
       this.pushQuery({
-        sql: `alter table ${quotedTableName} alter column "${colName}" drop default`,
+        sql: `alter table ${quotedTableName} alter column ${colName} drop default`,
         bindings: []
       });
       this.pushQuery({
-        sql: `alter table ${quotedTableName} alter column "${colName}" drop not null`,
+        sql: `alter table ${quotedTableName} alter column ${colName} drop not null`,
         bindings: []
       });
       this.pushQuery({
-        sql: `alter table ${quotedTableName} alter column "${colName}" type ${type} using ("${colName}"::${type})`,
+        sql: `alter table ${quotedTableName} alter column ${colName} type ${type} using (${colName}::${type})`,
         bindings: []
       });
 
       const defaultTo = col.modified['defaultTo'];
       if (defaultTo) {
         const modifier = col.defaultTo.apply(col, defaultTo);
         this.pushQuery({
-          sql: `alter table ${quotedTableName} alter column "${colName}" set ${modifier}`,
+          sql: `alter table ${quotedTableName} alter column ${colName} set ${modifier}`,
           bindings: []
         });
       }
 
       const nullable = col.modified['nullable'];
       if (nullable && nullable[0] === false) {
         this.pushQuery({
-          sql: `alter table ${quotedTableName} alter column "${colName}" set not null`,
+          sql: `alter table ${quotedTableName} alter column ${colName} set not null`,
           bindings: []
         });
       }

diff --git a/test/unit/schema/postgres.js b/test/unit/schema/postgres.js
@@ -798,6 +798,19 @@ describe("PostgreSQL SchemaBuilder", function() {
       expect(spy.secondCall.args).to.deep.equal(['email', 'email context']);
       expect(spy.thirdCall.args).to.deep.equal(['users', 'table context']);
     });
+
+    it('TableCompiler calls wrapIdentifier when altering column', function () {
+      client
+        .schemaBuilder()
+        .table('users', function (table) {
+          table.queryContext('table context');
+          table.string('email').notNull().alter().queryContext('email alter context');
+        })
+        .toSQL();
+
+      expect(spy.callCount).to.equal(3);
+      expect(spy.thirdCall.args).to.deep.equal(['email', 'email alter context']);
+    })
   });
 
 });