Repository: knex/knex
Pull request #4578 (2021-07-18 04:38:17)

Fixing posgres datetime and timestamp column created with wrong format
- Why:
See issue #4577 

- How: 
1. Change default useTz option of datetime and timezone to true in pg column compiler. 
2. Adding additional test to check when empty option object or option object wihout timezone is passed.



kibertoad: > Doesn't this change default values and hence is a breakibg change?
kibertoad: Isn't this still a breaking change, as it will invert values set by users already?
zeotuan: @kibertoad sorry the naming of variables was bad. Yeah i think so. I can change it so that it is not a breaking change anymore. 
kibertoad: It is cleaner, yes, just a bit too late to change it, would require a semver major and still likely to break stuff for people who don't read docs.
zeotuan: Alright i Changed it. It shouldn't invert value already set by user anymore. 

zeotuan: @kibertoad I changed it so it fixed the bug without making breaking changes. Is there any other change that i need to do? 

Patch:
diff --git a/lib/dialects/postgres/schema/pg-columncompiler.js b/lib/dialects/postgres/schema/pg-columncompiler.js
index a059e9e4..459b9093 100644
--- a/lib/dialects/postgres/schema/pg-columncompiler.js
+++ b/lib/dialects/postgres/schema/pg-columncompiler.js
@@ -71,10 +71,10 @@ class ColumnCompiler_PG extends ColumnCompiler {
     } else {
       useTz = !withoutTz;
     }
+    useTz = typeof useTz === 'boolean' ? useTz : true;
+    precision = precision ? '(' + precision + ')' : '';
 
-    return `${useTz ? 'timestamptz' : 'timestamp'}${
-      precision ? '(' + precision + ')' : ''
-    }`;
+    return `${useTz ? 'timestamptz' : 'timestamp'}${precision}`;
   }
 
   timestamp(withoutTz = false, precision) {
@@ -84,10 +84,10 @@ class ColumnCompiler_PG extends ColumnCompiler {
     } else {
       useTz = !withoutTz;
     }
+    useTz = typeof useTz === 'boolean' ? useTz : true;
+    precision = precision ? '(' + precision + ')' : '';
 
-    return `${useTz ? 'timestamptz' : 'timestamp'}${
-      precision ? '(' + precision + ')' : ''
-    }`;
+    return `${useTz ? 'timestamptz' : 'timestamp'}${precision}`;
   }
 
   // Modifiers:


Test patch:
diff --git a/test/unit/schema-builder/postgres.js b/test/unit/schema-builder/postgres.js
index 8d2252cb..1e9040ab 100644
--- a/test/unit/schema-builder/postgres.js
+++ b/test/unit/schema-builder/postgres.js
@@ -1155,6 +1155,19 @@ describe('PostgreSQL SchemaBuilder', function () {
     );
   });
 
+  it('adding default datetime', () => {
+    tableSql = client
+      .schemaBuilder()
+      .table('users', (table) => {
+        table.datetime('foo');
+      })
+      .toSQL();
+    equal(1, tableSql.length);
+    expect(tableSql[0].sql).to.equal(
+      'alter table "users" add column "foo" timestamptz'
+    );
+  });
+
   it('adding timestamp with timezone', () => {
     tableSql = client
       .schemaBuilder()
@@ -1168,6 +1181,19 @@ describe('PostgreSQL SchemaBuilder', function () {
     );
   });
 
+  it('adding datetime with timezone', () => {
+    tableSql = client
+      .schemaBuilder()
+      .table('users', (table) => {
+        table.datetime('foo', false);
+      })
+      .toSQL();
+    equal(1, tableSql.length);
+    expect(tableSql[0].sql).to.equal(
+      'alter table "users" add column "foo" timestamptz'
+    );
+  });
+
   it('adding timestamp without timezone', () => {
     tableSql = client
       .schemaBuilder()
@@ -1181,6 +1207,19 @@ describe('PostgreSQL SchemaBuilder', function () {
     );
   });
 
+  it('adding datetime without timezone', () => {
+    tableSql = client
+      .schemaBuilder()
+      .table('users', (table) => {
+        table.datetime('foo', true);
+      })
+      .toSQL();
+    equal(1, tableSql.length);
+    expect(tableSql[0].sql).to.equal(
+      'alter table "users" add column "foo" timestamp'
+    );
+  });
+
   it('adding timestamp with precision', () => {
     tableSql = client
       .schemaBuilder()
@@ -1194,6 +1233,19 @@ describe('PostgreSQL SchemaBuilder', function () {
     );
   });
 
+  it('adding datetime with precision', () => {
+    tableSql = client
+      .schemaBuilder()
+      .table('users', (table) => {
+        table.datetime('foo', undefined, 3);
+      })
+      .toSQL();
+    equal(1, tableSql.length);
+    expect(tableSql[0].sql).to.equal(
+      'alter table "users" add column "foo" timestamptz(3)'
+    );
+  });
+
   it('adding timestamp with options object', () => {
     tableSql = client
       .schemaBuilder()
@@ -1207,6 +1259,32 @@ describe('PostgreSQL SchemaBuilder', function () {
     );
   });
 
+  it('adding timestamp with options object but no timestamp', () => {
+    tableSql = client
+      .schemaBuilder()
+      .table('users', (table) => {
+        table.timestamp('foo', { precision: 3 });
+      })
+      .toSQL();
+    equal(1, tableSql.length);
+    expect(tableSql[0].sql).to.equal(
+      'alter table "users" add column "foo" timestamptz(3)'
+    );
+  });
+
+  it('adding timestamp with empty options object', () => {
+    tableSql = client
+      .schemaBuilder()
+      .table('users', (table) => {
+        table.timestamp('foo', {});
+      })
+      .toSQL();
+    equal(1, tableSql.length);
+    expect(tableSql[0].sql).to.equal(
+      'alter table "users" add column "foo" timestamptz'
+    );
+  });
+
   it('adding datetime with options object', () => {
     tableSql = client
       .schemaBuilder()
@@ -1220,6 +1298,32 @@ describe('PostgreSQL SchemaBuilder', function () {
     );
   });
 
+  it('adding datetime with options object but no timestamp', () => {
+    tableSql = client
+      .schemaBuilder()
+      .table('users', (table) => {
+        table.datetime('foo', { precision: 3 });
+      })
+      .toSQL();
+    equal(1, tableSql.length);
+    expect(tableSql[0].sql).to.equal(
+      'alter table "users" add column "foo" timestamptz(3)'
+    );
+  });
+
+  it('adding datetime with empty options object', () => {
+    tableSql = client
+      .schemaBuilder()
+      .table('users', (table) => {
+        table.datetime('foo', {});
+      })
+      .toSQL();
+    equal(1, tableSql.length);
+    expect(tableSql[0].sql).to.equal(
+      'alter table "users" add column "foo" timestamptz'
+    );
+  });
+
   it('adding timestamps', () => {
     tableSql = client
       .schemaBuilder()