Merge pull request #1431 from wubzz/bugfix/inconsistency_of_primary_a…

…nd_dropPrimary Ensure .primary() and .dropPrimary() are the same across all dialects.
knex · May 20, 2016 · 9118e05 · 9118e05
2 parents 25ca625 + f3898d7
commit 9118e05
Show file tree

Hide file tree

Showing 10 changed files with 113 additions and 29 deletions.
diff --git a/index.html b/index.html
@@ -2129,9 +2129,9 @@ <h3 id="Schema-Building">Schema Building:</h3>
 
 
     <p id="Schema-dropPrimary">
-      <b class="header">dropPrimary</b><code>table.dropPrimary()</code>
+      <b class="header">dropPrimary</b><code>table.dropPrimary([constraintName])</code>
       <br />
-      Drops the primary key constraint on a table.
+      Drops the primary key constraint on a table. Defaults to <tt>tablename_pkey</tt> unless <tt>constraintName</tt> is specified.
     </p>
 
     <h3 id="Chainable">Chainable Methods:</h3>
@@ -2150,10 +2150,11 @@ <h3 id="Chainable">Chainable Methods:</h3>
     </p>
 
     <p id="Chainable-primary">
-      <b class="header">primary</b><code>column.primary()</code>
+      <b class="header">primary</b><code>column.primary([constraintName])</code> / <code>table.primary(columns, [constraintName])</code>
       <br />
-      Sets the field as the primary key for the table.
-      To create a compound primary key, pass an array of column names: <code>table.primary(['column1', 'column2'])</code>.
+      When called on a single column it will set that column as the primary key for a table.
+      To create a compound primary key, pass an array of column names: <code>table.primary(['column1', 'column2'])</code>
+      <i>Constraint name defaults to <tt>tablename_pkey</tt> unless <tt>constraintName</tt> is specified.</i>
     </p>
 
     <p id="Chainable-unique">

diff --git a/src/dialects/mssql/schema/tablecompiler.js b/src/dialects/mssql/schema/tablecompiler.js
@@ -103,12 +103,12 @@ assign(TableCompiler_MSSQL.prototype, {
     this.pushQuery(`CREATE INDEX ${indexName} ON ${this.tableName()} (${this.formatter.columnize(columns)})`);
   },
 
-  primary (columns, indexName) {
-    indexName = indexName ? this.formatter.wrap(indexName) : this._indexCommand('primary', this.tableNameRaw, columns);
+  primary (columns, constraintName) {
+    constraintName = constraintName ? this.formatter.wrap(constraintName) : this._indexCommand('primary', this.tableNameRaw, columns);
     if (!this.forCreate) {
-      this.pushQuery(`ALTER TABLE ${this.tableName()} ADD PRIMARY KEY (${this.formatter.columnize(columns)})`);
+      this.pushQuery(`ALTER TABLE ${this.tableName()} ADD CONSTRAINT ${constraintName} PRIMARY KEY (${this.formatter.columnize(columns)})`);
     } else {
-      this.pushQuery(`CONSTRAINT ${indexName} PRIMARY KEY (${this.formatter.columnize(columns)})`);
+      this.pushQuery(`CONSTRAINT ${constraintName} PRIMARY KEY (${this.formatter.columnize(columns)})`);
     }
   },
 

diff --git a/src/dialects/mysql/schema/tablecompiler.js b/src/dialects/mysql/schema/tablecompiler.js
@@ -160,9 +160,9 @@ assign(TableCompiler_MySQL.prototype, {
     this.pushQuery(`alter table ${this.tableName()} add index ${indexName}(${this.formatter.columnize(columns)})`);
   },
 
-  primary(columns, indexName) {
-    indexName = indexName ? this.formatter.wrap(indexName) : this._indexCommand('primary', this.tableNameRaw, columns);
-    this.pushQuery(`alter table ${this.tableName()} add primary key ${indexName}(${this.formatter.columnize(columns)})`);
+  primary(columns, constraintName) {
+    constraintName = constraintName ? this.formatter.wrap(constraintName) : this.formatter.wrap(`${this.tableNameRaw}_pkey`);
+    this.pushQuery(`alter table ${this.tableName()} add primary key ${constraintName}(${this.formatter.columnize(columns)})`);
   },
 
   unique(columns, indexName) {

diff --git a/src/dialects/oracle/schema/tablecompiler.js b/src/dialects/oracle/schema/tablecompiler.js
@@ -64,12 +64,14 @@ assign(TableCompiler_Oracle.prototype, {
     return this.formatter.wrap(utils.generateCombinedName(type, tableName, columns));
   },
 
-  primary(columns) {
-    this.pushQuery(`alter table ${this.tableName()} add primary key (${this.formatter.columnize(columns)})`);
+  primary(columns, constraintName) {
+    constraintName = constraintName ? this.formatter.wrap(constraintName) : this.formatter.wrap(`${this.tableNameRaw}_pkey`);
+    this.pushQuery(`alter table ${this.tableName()} add constraint ${constraintName} primary key (${this.formatter.columnize(columns)})`);
   },
 
-  dropPrimary() {
-    this.pushQuery(`alter table ${this.tableName()} drop primary key`);
+  dropPrimary(constraintName) {
+    constraintName = constraintName ? this.formatter.wrap(constraintName) : this.formatter.wrap(this.tableNameRaw + '_pkey');
+    this.pushQuery(`alter table ${this.tableName()} drop constraint ${constraintName}`);
   },
 
   index(columns, indexName) {

diff --git a/src/dialects/postgres/schema/tablecompiler.js b/src/dialects/postgres/schema/tablecompiler.js
@@ -49,8 +49,9 @@ TableCompiler_PG.prototype.comment = function(comment) {
 // Indexes:
 // -------
 
-TableCompiler_PG.prototype.primary = function(columns) {
-  this.pushQuery(`alter table ${this.tableName()} add primary key (${this.formatter.columnize(columns)})`);
+TableCompiler_PG.prototype.primary = function(columns, constraintName) {
+  constraintName = constraintName ? this.formatter.wrap(constraintName) : this.formatter.wrap(`${this.tableNameRaw}_pkey`);
+  this.pushQuery(`alter table ${this.tableName()} add constraint ${constraintName} primary key (${this.formatter.columnize(columns)})`);
 };
 TableCompiler_PG.prototype.unique = function(columns, indexName) {
   indexName = indexName ? this.formatter.wrap(indexName) : this._indexCommand('unique', this.tableNameRaw, columns);
@@ -62,8 +63,8 @@ TableCompiler_PG.prototype.index = function(columns, indexName, indexType) {
   this.pushQuery(`create index ${indexName} on ${this.tableName()}${indexType && (` using ${indexType}`) || ''}` +
     ' (' + this.formatter.columnize(columns) + ')');
 };
-TableCompiler_PG.prototype.dropPrimary = function() {
-  const constraintName = this.formatter.wrap(this.tableNameRaw + '_pkey');
+TableCompiler_PG.prototype.dropPrimary = function(constraintName) {
+  constraintName = constraintName ? this.formatter.wrap(constraintName) : this.formatter.wrap(this.tableNameRaw + '_pkey');
   this.pushQuery(`alter table ${this.tableName()} drop constraint ${constraintName}`);
 };
 TableCompiler_PG.prototype.dropIndex = function(columns, indexName) {

diff --git a/test/integration/schema/index.js b/test/integration/schema/index.js
@@ -134,11 +134,11 @@ module.exports = function(knex) {
             table.integer('main').notNullable().primary();
             table.text('paragraph').defaultTo('Lorem ipsum Qui quis qui in.');
           }).testSql(function(tester) {
-            tester('mysql', ['create table `test_table_three` (`main` int not null, `paragraph` text) default character set utf8 engine = InnoDB','alter table `test_table_three` add primary key `test_table_three_main_primary`(`main`)']);
-            tester('pg', ['create table "test_table_three" ("main" integer not null, "paragraph" text default \'Lorem ipsum Qui quis qui in.\')','alter table "test_table_three" add primary key ("main")']);
+            tester('mysql', ['create table `test_table_three` (`main` int not null, `paragraph` text) default character set utf8 engine = InnoDB','alter table `test_table_three` add primary key `test_table_three_pkey`(`main`)']);
+            tester('pg', ['create table "test_table_three" ("main" integer not null, "paragraph" text default \'Lorem ipsum Qui quis qui in.\')','alter table "test_table_three" add constraint "test_table_three_pkey" primary key ("main")']);
             tester('sqlite3', ['create table "test_table_three" ("main" integer not null, "paragraph" text default \'Lorem ipsum Qui quis qui in.\', primary key ("main"))']);
-            tester('oracle', ['create table "test_table_three" ("main" integer not null, "paragraph" clob default \'Lorem ipsum Qui quis qui in.\')','alter table "test_table_three" add primary key ("main")']);
-            tester('mssql', ['CREATE TABLE [test_table_three] ([main] int not null, [paragraph] nvarchar(max), CONSTRAINT [test_table_three_main_primary] PRIMARY KEY ([main]))']);
+            tester('oracle', ['create table "test_table_three" ("main" integer not null, "paragraph" clob default \'Lorem ipsum Qui quis qui in.\')','alter table "test_table_three" add constraint "test_table_three_pkey" primary key ("main")']);
+            tester('mssql', ['CREATE TABLE [test_table_three] ([main] int not null, [paragraph] nvarchar(max), CONSTRAINT [test_table_three_pkey] PRIMARY KEY ([main]))']);
           });
       });
 
@@ -467,5 +467,34 @@ module.exports = function(knex) {
       });
     });
 
+
+    //Unit tests checks SQL -- This will test running those queries, no hard assertions here.
+    it('#1430 - .primary() & .dropPrimary() same for all dialects', function() {
+      if(/sqlite/i.test(knex.client.dialect)) {
+        return Promise.resolve();
+      }
+      var constraintName = 'testconstraintname';
+      var tableName = 'primarytest';
+      return knex.transaction(function(tr) {
+        return tr.schema.dropTableIfExists(tableName)
+        .then(function() {
+            return tr.schema.createTable(tableName, function(table) {
+              table.string('test').primary(constraintName);
+              table.string('test2');
+            })
+          })
+          .then(function() {
+            return tr.schema.table(tableName, function(table) {
+              table.dropPrimary(constraintName);
+            })
+          })
+          .then(function() {
+            return tr.schema.table(tableName, function(table) {
+              table.primary(['test', 'test2'], constraintName)
+            })
+          });
+      });
+    });
+
   });
 };
diff --git a/test/unit/schema/mssql.js b/test/unit/schema/mssql.js
@@ -161,7 +161,7 @@ describe("MSSQL SchemaBuilder", function() {
     }).toSQL();
 
     equal(1, tableSql.length);
-    expect(tableSql[0].sql).to.equal('ALTER TABLE [users] ADD PRIMARY KEY ([foo])');
+    expect(tableSql[0].sql).to.equal('ALTER TABLE [users] ADD CONSTRAINT [bar] PRIMARY KEY ([foo])');
   });
 
   it('test adding unique key', function() {
@@ -479,4 +479,18 @@ describe("MSSQL SchemaBuilder", function() {
     expect(tableSql[0].sql).to.equal('CREATE TABLE [default_raw_test] ([created_at] datetime default GETDATE())');
   });
 
+
+  it('#1430 - .primary & .dropPrimary takes columns and constraintName', function() {
+    tableSql = client.schemaBuilder().table('users', function(t) {
+      t.primary(['test1', 'test2'], 'testconstraintname');
+    }).toSQL();
+    expect(tableSql[0].sql).to.equal('ALTER TABLE [users] ADD CONSTRAINT [testconstraintname] PRIMARY KEY ([test1], [test2])');
+
+    tableSql = client.schemaBuilder().createTable('users', function(t) {
+      t.string('test').primary('testconstraintname');
+    }).toSQL();
+
+    expect(tableSql[0].sql).to.equal('CREATE TABLE [users] ([test] nvarchar(255), CONSTRAINT [testconstraintname] PRIMARY KEY ([test]))');
+  });
+
 });
diff --git a/test/unit/schema/mysql.js b/test/unit/schema/mysql.js
@@ -496,6 +496,19 @@ describe(dialect + " SchemaBuilder", function() {
     expect(tableSql[0].sql).to.equal('create table `users` (`username` varchar(255)) engine = myISAM');
   });
 
+  it('#1430 - .primary & .dropPrimary takes columns and constraintName', function() {
+    tableSql = client.schemaBuilder().table('users', function(t) {
+      t.primary(['test1', 'test2'], 'testconstraintname');
+    }).toSQL();
+    expect(tableSql[0].sql).to.equal('alter table `users` add primary key `testconstraintname`(`test1`, `test2`)');
+
+    tableSql = client.schemaBuilder().createTable('users', function(t) {
+      t.string('test').primary('testconstraintname');
+    }).toSQL();
+
+    expect(tableSql[1].sql).to.equal('alter table `users` add primary key `testconstraintname`(`test`)');
+  });
+
 });
 
 

diff --git a/test/unit/schema/oracle.js b/test/unit/schema/oracle.js
@@ -83,7 +83,7 @@ describe("Oracle SchemaBuilder", function() {
     }).toSQL();
 
     equal(1, tableSql.length);
-    expect(tableSql[0].sql).to.equal('alter table "users" drop primary key');
+    expect(tableSql[0].sql).to.equal('alter table "users" drop constraint "users_pkey"');
   });
 
   it('test drop unique', function() {
@@ -161,7 +161,7 @@ describe("Oracle SchemaBuilder", function() {
     }).toSQL();
 
     equal(1, tableSql.length);
-    expect(tableSql[0].sql).to.equal('alter table "users" add primary key ("foo")');
+    expect(tableSql[0].sql).to.equal('alter table "users" add constraint "bar" primary key ("foo")');
   });
 
   it('test adding unique key', function() {
@@ -492,5 +492,16 @@ describe("Oracle SchemaBuilder", function() {
     equal(1, tableSql.length);
     expect(tableSql[0].sql).to.equal('alter table "composite_key_test" drop constraint "ckt_unique"');
   });
+  it('#1430 - .primary & .dropPrimary takes columns and constraintName', function() {
+    tableSql = client.schemaBuilder().table('users', function(t) {
+      t.primary(['test1', 'test2'], 'testconstraintname');
+    }).toSQL();
+    expect(tableSql[0].sql).to.equal('alter table "users" add constraint "testconstraintname" primary key ("test1", "test2")');
+
+    tableSql = client.schemaBuilder().createTable('users', function(t) {
+      t.string('test').primary('testconstraintname');
+    }).toSQL();
 
+    expect(tableSql[1].sql).to.equal('alter table "users" add constraint "testconstraintname" primary key ("test")');
+  });
 });
diff --git a/test/unit/schema/postgres.js b/test/unit/schema/postgres.js
@@ -161,7 +161,7 @@ describe("PostgreSQL SchemaBuilder", function() {
       table.primary('foo');
     }).toSQL();
     equal(1, tableSql.length);
-    expect(tableSql[0].sql).to.equal('alter table "users" add primary key ("foo")');
+    expect(tableSql[0].sql).to.equal('alter table "users" add constraint "users_pkey" primary key ("foo")');
   });
 
   it("adding primary key fluently", function() {
@@ -170,7 +170,7 @@ describe("PostgreSQL SchemaBuilder", function() {
     }).toSQL();
     equal(2, tableSql.length);
     expect(tableSql[0].sql).to.equal('create table "users" ("name" varchar(255))');
-    expect(tableSql[1].sql).to.equal('alter table "users" add primary key ("name")');
+    expect(tableSql[1].sql).to.equal('alter table "users" add constraint "users_pkey" primary key ("name")');
   });
 
   it("adding foreign key", function() {
@@ -515,4 +515,17 @@ describe("PostgreSQL SchemaBuilder", function() {
     expect(tableSql[0].sql).to.equal('create table "users" ("username" varchar(255))');
   });
 
+  it('#1430 - .primary & .dropPrimary takes columns and constraintName', function() {
+    tableSql = client.schemaBuilder().table('users', function(t) {
+      t.primary(['test1', 'test2'], 'testconstraintname');
+    }).toSQL();
+    expect(tableSql[0].sql).to.equal('alter table "users" add constraint "testconstraintname" primary key ("test1", "test2")');
+
+    tableSql = client.schemaBuilder().createTable('users', function(t) {
+      t.string('test').primary('testconstraintname');
+    }).toSQL();
+
+    expect(tableSql[1].sql).to.equal('alter table "users" add constraint "testconstraintname" primary key ("test")');
+  });
+
 });