Repository: knex/knex
Pull request #5233 (2022-06-20 18:44:38)

feat: inline primary key creation for postgres flavours
This is a continuation of my work in #5212. It's aimed at really fixing #5211

* #5212
* #5211

#5212 implemented `uuid(<id>, { primaryKey: true })` for allowing to use `PRIMARY KEY` on the field type in the column declaration. This aims at adding support for a distinct `PRIMARY KEY` constraint during table creating.

Both of these solutions want to make life easier for folx using CockroachDB which autogenerates a `rowid` primary if none is specified during initial table creating.

I'm creating this as a draft PR, I'll finalize it once the accompanying docs pr is also done and CI is green.

Please let me know if this is a feasible way to go forward and if there is some special consideration needed for any non-postgresql postgres flavours (i don't know them well). 

# tasks

* [x] take a closer look at why some non-postgres flavours are failing
* [x] add guards for new tests and their non-postgres cases
* [x] figure out why [call on table with columns and name](https://github.com/hairmare/knex/blob/417abf9987bbced5917d9192f3214e0de019f095/test/unit/schema-builder/postgres.js#L2075-L2086) is failing with a timeout *after* the tests expectations are met and it succeeded


Patch:
diff --git a/lib/dialects/postgres/schema/pg-tablecompiler.js b/lib/dialects/postgres/schema/pg-tablecompiler.js
index 3bb2d7d9..d2eeb980 100644
--- a/lib/dialects/postgres/schema/pg-tablecompiler.js
+++ b/lib/dialects/postgres/schema/pg-tablecompiler.js
@@ -44,7 +44,10 @@ class TableCompiler_PG extends TableCompiler {
     const createStatement = ifNot
       ? 'create table if not exists '
       : 'create table ';
-    const columnsSql = ' (' + columns.sql.join(', ') + this._addChecks() + ')';
+    const columnsSql = ` (${columns.sql.join(', ')}${
+      this.primaryKeys() || ''
+    }${this._addChecks()})`;
+
     let sql =
       createStatement +
       this.tableName() +
@@ -65,6 +68,28 @@ class TableCompiler_PG extends TableCompiler {
     if (hasComment) this.comment(this.single.comment);
   }
 
+  primaryKeys() {
+    const pks = (this.grouped.alterTable || []).filter(
+      (k) => k.method === 'primary'
+    );
+    if (pks.length > 0 && pks[0].args.length > 0) {
+      const columns = pks[0].args[0];
+      let constraintName = pks[0].args[1] || '';
+      let deferrable;
+      if (isObject(constraintName)) {
+        ({ constraintName, deferrable } = constraintName);
+      }
+      deferrable = deferrable ? ` deferrable initially ${deferrable}` : '';
+      constraintName = constraintName
+        ? this.formatter.wrap(constraintName)
+        : this.formatter.wrap(`${this.tableNameRaw}_pkey`);
+
+      return `, constraint ${constraintName} primary key (${this.formatter.columnize(
+        columns
+      )})${deferrable}`;
+    }
+  }
+
   addColumns(columns, prefix, colCompilers) {
     if (prefix === this.alterColumnsPrefix) {
       // alter columns
@@ -153,11 +178,13 @@ class TableCompiler_PG extends TableCompiler {
     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
-      )})${deferrable}`
-    );
+    if (this.method !== 'create' && this.method !== 'createIfNot') {
+      this.pushQuery(
+        `alter table ${this.tableName()} add constraint ${constraintName} primary key (${this.formatter.columnize(
+          columns
+        )})${deferrable}`
+      );
+    }
   }
 
   unique(columns, indexName) {
diff --git a/package.json b/package.json
index 9667ad46..a2bd539f 100644
--- a/package.json
+++ b/package.json
@@ -60,7 +60,7 @@
     "prepublishOnly": "npm run clean && npm run build"
   },
   "dependencies": {
-    "colorette": "2.0.18",
+    "colorette": "2.0.19",
     "commander": "^9.1.0",
     "debug": "4.3.4",
     "escalade": "^3.1.1",
diff --git a/types/index.d.ts b/types/index.d.ts
index 65a40027..26edbb25 100644
--- a/types/index.d.ts
+++ b/types/index.d.ts
@@ -502,6 +502,7 @@ export declare namespace Knex {
     | Array<Date>
     | Array<boolean>
     | Buffer
+    | object
     | Knex.Raw;
 
   interface ValueDict extends Dict<Value | Knex.QueryBuilder> {}


Test patch:
diff --git a/test/integration2/schema/misc.spec.js b/test/integration2/schema/misc.spec.js
index 715bd528..20db0132 100644
--- a/test/integration2/schema/misc.spec.js
+++ b/test/integration2/schema/misc.spec.js
@@ -483,25 +483,54 @@ describe('Schema (misc)', () => {
         });
 
         describe('uuid types - postgres', () => {
-          before(async () => {
-            await knex.schema.createTable('uuid_column_test', (table) => {
-              table.uuid('id', { primaryKey: true });
-            });
+          after(async () => {
+            if (isPgBased(knex)) {
+              await knex.schema.dropTable('uuid_column_test');
+            }
           });
 
-          after(async () => {
-            await knex.schema.dropTable('uuid_column_test');
+          it('creates a uuid column as primary using fluid syntax', async function () {
+            if (!isPgBased(knex)) {
+              return this.skip();
+            }
+            const table_name = 'uuid_column_test';
+            const expected_column = 'id';
+            const expected_type = 'uuid';
+
+            await knex.schema.dropTableIfExists(table_name);
+            await knex.schema.createTable(table_name, (table) => {
+              table.uuid('id').primary();
+            });
+
+            const cols = await knex.raw(
+              `select c.column_name, c.data_type
+                     from INFORMATION_SCHEMA.COLUMNS c
+                     join INFORMATION_SCHEMA.KEY_COLUMN_USAGE cu
+                     on (c.table_name = cu.table_name and c.column_name = cu.column_name)
+                     where c.table_name = ?
+                     and (cu.constraint_name like '%_pkey' or cu.constraint_name = 'primary')`,
+              table_name
+            );
+            const column_name = cols.rows[0].column_name;
+            const column_type = cols.rows[0].data_type;
+
+            expect(column_name).to.equal(expected_column);
+            expect(column_type).to.equal(expected_type);
           });
 
           it('#5211 - creates an uuid column as primary key', async function () {
             if (!isPgBased(knex)) {
               return this.skip();
             }
-
             const table_name = 'uuid_column_test';
             const expected_column = 'id';
             const expected_type = 'uuid';
 
+            await knex.schema.dropTableIfExists(table_name);
+            await knex.schema.createTable(table_name, (table) => {
+              table.uuid('id', { primaryKey: true });
+            });
+
             const cols = await knex.raw(
               `select c.column_name, c.data_type
                      from INFORMATION_SCHEMA.COLUMNS c
@@ -862,8 +891,7 @@ describe('Schema (misc)', () => {
               tester(
                 ['pg', 'cockroachdb'],
                 [
-                  'create table "test_table_three" ("main" integer not null, "paragraph" text default \'Lorem ipsum Qui quis qui in.\', "metadata" json default \'{"a":10}\', "details" jsonb default \'{"b":{"d":20}}\')',
-                  'alter table "test_table_three" add constraint "test_table_three_pkey" primary key ("main")',
+                  'create table "test_table_three" ("main" integer not null, "paragraph" text default \'Lorem ipsum Qui quis qui in.\', "metadata" json default \'{"a":10}\', "details" jsonb default \'{"b":{"d":20}}\', constraint "test_table_three_pkey" primary key ("main"))',
                 ]
               );
               tester('pg-redshift', [
diff --git a/test/unit/schema-builder/postgres.js b/test/unit/schema-builder/postgres.js
index f27f0417..a77f68c7 100644
--- a/test/unit/schema-builder/postgres.js
+++ b/test/unit/schema-builder/postgres.js
@@ -150,6 +150,20 @@ describe('PostgreSQL SchemaBuilder', function () {
     );
   });
 
+  it('should create a table with inline uuid primary key creation', function () {
+    tableSql = client
+      .schemaBuilder()
+      .createTable('uuids', function (table) {
+        table.uuid('id').primary();
+      })
+      .toSQL();
+
+    equal(1, tableSql.length);
+    expect(tableSql[0].sql).to.equal(
+      'create table "uuids" ("id" uuid, constraint "uuids_pkey" primary key ("id"))'
+    );
+  });
+
   it('basic alter table', function () {
     tableSql = client
       .schemaBuilder()
@@ -547,6 +561,19 @@ describe('PostgreSQL SchemaBuilder', function () {
     );
   });
 
+  it('drop primary takes constraint name', function () {
+    tableSql = client
+      .schemaBuilder()
+      .table('users', function (table) {
+        table.dropPrimary('testconstraintname');
+      })
+      .toSQL();
+    equal(1, tableSql.length);
+    expect(tableSql[0].sql).to.equal(
+      'alter table "users" drop constraint "testconstraintname"'
+    );
+  });
+
   it('drop unique', function () {
     tableSql = client
       .schemaBuilder()
@@ -688,9 +715,9 @@ describe('PostgreSQL SchemaBuilder', function () {
         });
       })
       .toSQL();
-    equal(2, tableSql.length);
-    expect(tableSql[1].sql).to.equal(
-      'alter table "person" add constraint "user_id_primary" primary key ("user_id") deferrable initially immediate'
+    equal(1, tableSql.length);
+    expect(tableSql[0].sql).to.equal(
+      'create table "person" ("user_id" integer, constraint "user_id_primary" primary key ("user_id") deferrable initially immediate)'
     );
   });
 
@@ -733,12 +760,9 @@ describe('PostgreSQL SchemaBuilder', function () {
         table.string('name').primary();
       })
       .toSQL();
-    equal(2, tableSql.length);
+    equal(1, tableSql.length);
     expect(tableSql[0].sql).to.equal(
-      'create table "users" ("name" varchar(255))'
-    );
-    expect(tableSql[1].sql).to.equal(
-      'alter table "users" add constraint "users_pkey" primary key ("name")'
+      'create table "users" ("name" varchar(255), constraint "users_pkey" primary key ("name"))'
     );
   });
 
@@ -1992,12 +2016,15 @@ describe('PostgreSQL SchemaBuilder', function () {
   it('#1430 - .primary & .dropPrimary takes columns and constraintName', function () {
     tableSql = client
       .schemaBuilder()
-      .table('users', function (t) {
+      .createTable('users', function (t) {
+        t.string('test1');
+        t.string('test2');
         t.primary(['test1', 'test2'], 'testconstraintname');
       })
       .toSQL();
+    equal(1, tableSql.length);
     expect(tableSql[0].sql).to.equal(
-      'alter table "users" add constraint "testconstraintname" primary key ("test1", "test2")'
+      'create table "users" ("test1" varchar(255), "test2" varchar(255), constraint "testconstraintname" primary key ("test1", "test2"))'
     );
 
     tableSql = client
@@ -2007,11 +2034,59 @@ describe('PostgreSQL SchemaBuilder', function () {
       })
       .toSQL();
 
-    expect(tableSql[1].sql).to.equal(
-      'alter table "users" add constraint "testconstraintname" primary key ("test")'
+    expect(tableSql[0].sql).to.equal(
+      'create table "users" ("test" varchar(255), constraint "testconstraintname" primary key ("test"))'
     );
   });
 
+  describe('alter with primary', function () {
+    it('liquid argument', function () {
+      tableSql = client
+        .schemaBuilder()
+        .alterTable('users', function (t) {
+          t.string('test').primary();
+        })
+        .toSQL();
+
+      equal(2, tableSql.length);
+      expect(tableSql[0].sql).to.equal(
+        'alter table "users" add column "test" varchar(255)'
+      );
+      expect(tableSql[1].sql).to.equal(
+        'alter table "users" add constraint "users_pkey" primary key ("test")'
+      );
+    });
+    it('liquid argument with name', function () {
+      tableSql = client
+        .schemaBuilder()
+        .alterTable('users', function (t) {
+          t.string('test').primary('testname');
+        })
+        .toSQL();
+
+      equal(2, tableSql.length);
+      expect(tableSql[0].sql).to.equal(
+        'alter table "users" add column "test" varchar(255)'
+      );
+      expect(tableSql[1].sql).to.equal(
+        'alter table "users" add constraint "testname" primary key ("test")'
+      );
+    });
+    it('call on table with columns and name', function () {
+      tableSql = client
+        .schemaBuilder()
+        .alterTable('users', function (t) {
+          t.primary(['test1', 'test2'], 'testconstraintname');
+        })
+        .toSQL();
+
+      equal(1, tableSql.length);
+      expect(tableSql[0].sql).to.equal(
+        'alter table "users" add constraint "testconstraintname" primary key ("test1", "test2")'
+      );
+    });
+  });
+
   describe('queryContext', function () {
     let spy;
     let originalWrapIdentifier;