ColumnDefinition: Make timestamp columns NULL by default

nwoltman · nwoltman · commit dfcd030d52ff · 2017-03-25T23:57:03.000-04:00
This makes timestamp column definitions consistent with how all the other mysql-plus column definitions work. This also fixes a bug where some timestamp column definitions would cause unnecessary ALTER migrations. Fixes #3
diff --git a/README.md b/README.md
@@ -1221,6 +1221,48 @@ Compatible types:
 + `datetime`
 + `timestamp`
 
+#### TimestampColumnDefinition
+
+Compatible types:
+
++ `timestamp`
+
+There aren't any extra methods on this type, but there are some things you should be aware of with `timestamp` columns:
+
+##### NULL Timestamps
+
+Normally, timestamp columns are `NOT NULL` by default, however, mysql-plus defines timestamp columns to be `NULL` by default to keep column definition semantics consistent. Therefore, the following column definition:
+
+```js
+{
+  ts: ColTypes.timestamp(),
+}
+```
+
+would define a column with this SQL:
+
+```sql
+`ts` timestamp NULL DEFAULT NULL
+```
+
+##### Timestamps' DEFAULT value
+
+If you define a timestamp column and use the `notNull()` method, the column's `DEFAULT` value will be set to `CURRENT_TIMESTAMP`. So the following:
+
+```js
+{
+  ts: ColTypes.timestamp().notNull(), // equivalent to `ColTypes.timestamp().notNull().defaultCurrentTimestamp()`
+}
+```
+
+would define a column with this SQL:
+
+```sql
+`ts` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP
+```
+
+Normally if the `DEFAULT` is unspecified, MySQL uses `CURRENT_TIMESTAMP` as the `DEFAULT` value of only the first timestamp column and `'0000-00-00 00:00:00'` for subsequent columns, but `mysql-plus` uses `CURRENT_TIMESTAMP` for all timestamp columns for consistency.
+
 #### GeometricalColumnDefinition
 
 Methods:
diff --git a/jsdoc2md/README.hbs b/jsdoc2md/README.hbs
@@ -428,6 +428,48 @@ Compatible types:
 + `datetime`
 + `timestamp`
 
+#### TimestampColumnDefinition
+
+Compatible types:
+
++ `timestamp`
+
+There aren't any extra methods on this type, but there are some things you should be aware of with `timestamp` columns:
+
+##### NULL Timestamps
+
+Normally, timestamp columns are `NOT NULL` by default, however, mysql-plus defines timestamp columns to be `NULL` by default to keep column definition semantics consistent. Therefore, the following column definition:
+
+```js
+{
+  ts: ColTypes.timestamp(),
+}
+```
+
+would define a column with this SQL:
+
+```sql
+`ts` timestamp NULL DEFAULT NULL
+```
+
+##### Timestamps' DEFAULT value
+
+If you define a timestamp column and use the `notNull()` method, the column's `DEFAULT` value will be set to `CURRENT_TIMESTAMP`. So the following:
+
+```js
+{
+  ts: ColTypes.timestamp().notNull(), // equivalent to `ColTypes.timestamp().notNull().defaultCurrentTimestamp()`
+}
+```
+
+would define a column with this SQL:
+
+```sql
+`ts` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP
+```
+
+Normally if the `DEFAULT` is unspecified, MySQL uses `CURRENT_TIMESTAMP` as the `DEFAULT` value of only the first timestamp column and `'0000-00-00 00:00:00'` for subsequent columns, but `mysql-plus` uses `CURRENT_TIMESTAMP` for all timestamp columns for consistency.
+
 #### GeometricalColumnDefinition
 
 Methods:
diff --git a/lib/ColumnDefinitions/ColumnDefinition.js b/lib/ColumnDefinitions/ColumnDefinition.js
@@ -18,11 +18,11 @@ class ColumnDefinition {
     this._baseType = type;
     if (m == null) {
       this.$hasLength = false;
+      this._type = type;
     } else {
       this.$hasLength = true;
-      type += '(' + mysql.escape(m) + (d == null ? '' : ',' + mysql.escape(d)) + ')';
+      this._type = type + '(' + mysql.escape(m) + (d == null ? '' : ',' + mysql.escape(d)) + ')';
     }
-    this.$type = type;
     this.$primaryKey = false;
     this.$unique = false;
     this.$index = false;
@@ -68,12 +68,12 @@ class ColumnDefinition {
   }
 
   $equals(columnDefinition) {
-    var thisType = this.$type;
-    var otherType = columnDefinition.$type;
+    var thisType = this._type;
+    var otherType = columnDefinition._type;
 
     if (this.$hasLength && columnDefinition.$hasLength) {
-      thisType = this.$type;
-      otherType = columnDefinition.$type;
+      thisType = this._type;
+      otherType = columnDefinition._type;
     } else {
       thisType = this._baseType;
       otherType = columnDefinition._baseType;
@@ -96,7 +96,7 @@ class ColumnDefinition {
   }
 
   $toSQL() {
-    var sql = this.__getType();
+    var sql = this._type + this.__getExtendedType();
 
     if (this._notNull) {
       sql += ' NOT NULL';
@@ -108,8 +108,8 @@ class ColumnDefinition {
     return sql;
   }
 
-  __getType() {
-    return this.$type;
+  __getExtendedType() {
+    return '';
   }
 }
 
diff --git a/lib/ColumnDefinitions/NumericColumnDefinition.js b/lib/ColumnDefinitions/NumericColumnDefinition.js
@@ -33,18 +33,18 @@ class NumericColumnDefinition extends ColumnDefinition {
       this._autoIncrement === columnDefinition._autoIncrement;
   }
 
-  __getType() {
-    var type = this.$type;
+  __getExtendedType() {
+    var extendedType = '';
     if (this._unsigned) {
-      type += ' unsigned';
+      extendedType += ' unsigned';
     }
     if (this._zerofill) {
-      type += ' zerofill';
+      extendedType += ' zerofill';
     }
     if (this._autoIncrement) {
-      type += ' AUTO_INCREMENT';
+      extendedType += ' AUTO_INCREMENT';
     }
-    return type;
+    return extendedType;
   }
 }
 
diff --git a/lib/ColumnDefinitions/TextColumnDefinition.js b/lib/ColumnDefinitions/TextColumnDefinition.js
@@ -48,15 +48,15 @@ class TextColumnDefinition extends ColumnDefinition {
     return true;
   }
 
-  __getType() {
-    var type = this.$type;
+  __getExtendedType() {
+    var extendedType = '';
     if (this._charset) {
-      type += ' CHARACTER SET ' + this._charset;
+      extendedType += ' CHARACTER SET ' + this._charset;
     }
     if (this._collate) {
-      type += ' COLLATE ' + this._collate;
+      extendedType += ' COLLATE ' + this._collate;
     }
-    return type;
+    return extendedType;
   }
 }
 
diff --git a/lib/ColumnDefinitions/TimestampColumnDefinition.js b/lib/ColumnDefinitions/TimestampColumnDefinition.js
@@ -0,0 +1,34 @@
+'use strict';
+
+const UpdatableTimeColumnDefinition = require('./UpdatableTimeColumnDefinition');
+
+class TimestampColumnDefinition extends UpdatableTimeColumnDefinition {
+  constructor(m) {
+    super('timestamp', m);
+    this._allowNull = true;
+  }
+
+  notNull() {
+    this._allowNull = false;
+    return super.notNull().defaultCurrentTimestamp();
+  }
+
+  default(value) {
+    if (value === 0) {
+      value = '0000-00-00 00:00:00';
+    }
+    return super.default(value);
+  }
+
+  $equals(columnDefinition) {
+    return super.$equals(columnDefinition) &&
+      this._allowNull === columnDefinition._allowNull;
+  }
+
+  __getExtendedType() {
+    const extendedType = this._allowNull ? ' NULL' : '';
+    return extendedType + super.__getExtendedType();
+  }
+}
+
+module.exports = TimestampColumnDefinition;
diff --git a/lib/ColumnDefinitions/UpdatableTimeColumnDefinition.js b/lib/ColumnDefinitions/UpdatableTimeColumnDefinition.js
@@ -22,12 +22,8 @@ class UpdatableTimeColumnDefinition extends ColumnDefinition {
       this._onUpdateCurrentTimestamp === columnDefinition._onUpdateCurrentTimestamp;
   }
 
-  __getType() {
-    var type = this.$type;
-    if (this._onUpdateCurrentTimestamp) {
-      type += ' ON UPDATE CURRENT_TIMESTAMP';
-    }
-    return type;
+  __getExtendedType() {
+    return this._onUpdateCurrentTimestamp ? ' ON UPDATE CURRENT_TIMESTAMP' : '';
   }
 }
 
diff --git a/lib/ColumnDefinitions/index.js b/lib/ColumnDefinitions/index.js
@@ -10,6 +10,7 @@ const ColumnDefinition = require('./ColumnDefinition');
 const GeometricalColumnDefinition = require('./GeometricalColumnDefinition');
 const NumericColumnDefinition = require('./NumericColumnDefinition');
 const TextColumnDefinition = require('./TextColumnDefinition');
+const TimestampColumnDefinition = require('./TimestampColumnDefinition');
 const UpdatableTimeColumnDefinition = require('./UpdatableTimeColumnDefinition');
 
 const ColumnDefinitions = {
@@ -65,7 +66,7 @@ const ColumnDefinitions = {
     return new UpdatableTimeColumnDefinition('datetime', m);
   },
   timestamp(m) {
-    return new UpdatableTimeColumnDefinition('timestamp', m);
+    return new TimestampColumnDefinition(m);
   },
   time(m) {
     return new ColumnDefinition('time', m);
diff --git a/test/integration/MySQLPlus.test.js b/test/integration/MySQLPlus.test.js
@@ -526,6 +526,29 @@ describe('MySQLPlus', function() {
     '  `e` char(1) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL\n' +
     ') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_520_ci';
 
+  const timestampTableName = 'timestamp_table';
+  const timestampTableSchema = {
+    columns: {
+      a: ColTypes.timestamp().notNull(),
+      b: ColTypes.timestamp().notNull().defaultCurrentTimestamp(),
+      c: ColTypes.timestamp().notNull().default(0),
+      d: ColTypes.timestamp(),
+      e: ColTypes.timestamp().default(null),
+      f: ColTypes.timestamp().default(0),
+      g: ColTypes.timestamp().default('2017-03-25 12:46:05'),
+    },
+  };
+  const timestampTableExpectedSQL =
+    'CREATE TABLE `timestamp_table` (\n' +
+    '  `a` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,\n' +
+    '  `b` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,\n' +
+    "  `c` timestamp NOT NULL DEFAULT '0000-00-00 00:00:00',\n" +
+    '  `d` timestamp NULL DEFAULT NULL,\n' +
+    '  `e` timestamp NULL DEFAULT NULL,\n' +
+    "  `f` timestamp NULL DEFAULT '0000-00-00 00:00:00',\n" +
+    "  `g` timestamp NULL DEFAULT '2017-03-25 12:46:05'\n" +
+    ') ENGINE=InnoDB DEFAULT CHARSET=utf8';
+
 
   describe('when creating new tables', () => {
 
@@ -543,6 +566,7 @@ describe('MySQLPlus', function() {
       pool.defineTable(foreignKeysTableName, foreignKeysTableSchema);
       pool.defineTable(optionsTableName, optionsTableSchema);
       pool.defineTable(textTableName, textTableSchema);
+      pool.defineTable(timestampTableName, timestampTableSchema);
       pool.sync(done);
     });
 
@@ -629,6 +653,13 @@ describe('MySQLPlus', function() {
         result[0]['Create Table'].should.equal(textTableExpectedSQL);
         cb10();
       });
+
+      const cbTimestamp = cbManager.registerCallback();
+      pool.query(`SHOW CREATE TABLE \`${timestampTableName}\``, (err, result) => {
+        if (err) throw err;
+        result[0]['Create Table'].should.equal(timestampTableExpectedSQL);
+        cbTimestamp();
+      });
     });
 
   });
@@ -651,6 +682,7 @@ describe('MySQLPlus', function() {
       pool.defineTable(foreignKeysTableName, foreignKeysTableSchema);
       pool.defineTable(optionsTableName, optionsTableSchema);
       pool.defineTable(textTableName, textTableSchema);
+      pool.defineTable(timestampTableName, timestampTableSchema);
       pool.sync(done);
     });
 
@@ -742,6 +774,13 @@ describe('MySQLPlus', function() {
         result[0]['Create Table'].should.equal(textTableExpectedSQL);
         cb10();
       });
+
+      const cbTimestamp = cbManager.registerCallback();
+      pool.query(`SHOW CREATE TABLE \`${timestampTableName}\``, (err, result) => {
+        if (err) throw err;
+        result[0]['Create Table'].should.equal(timestampTableExpectedSQL);
+        cbTimestamp();
+      });
     });
 
   });
@@ -764,6 +803,7 @@ describe('MySQLPlus', function() {
       pool.defineTable(foreignKeysTableName, foreignKeysTableSchema);
       pool.defineTable(optionsTableName, optionsTableSchema);
       pool.defineTable(textTableName, textTableSchema);
+      pool.defineTable(timestampTableName, timestampTableSchema);
       pool.sync(done);
     });
 
@@ -850,6 +890,13 @@ describe('MySQLPlus', function() {
         result[0]['Create Table'].should.equal(textTableExpectedSQL);
         cb10();
       });
+
+      const cbTimestamp = cbManager.registerCallback();
+      pool.query(`SHOW CREATE TABLE \`${timestampTableName}\``, (err, result) => {
+        if (err) throw err;
+        result[0]['Create Table'].should.equal(timestampTableExpectedSQL);
+        cbTimestamp();
+      });
     });
 
   });
diff --git a/test/unit/ColumnDefinitions.test.js b/test/unit/ColumnDefinitions.test.js

Original file line number	Diff line number	Diff line change
`@@ -33,18 +33,18 @@ class NumericColumnDefinition extends ColumnDefinition {`
`33`	`33`	`this._autoIncrement === columnDefinition._autoIncrement;`
`34`	`34`	`}`
`35`	`35`
`36`		`- __getType() {`
`37`		`- var type = this.$type;`
	`36`	`+ __getExtendedType() {`
	`37`	`+ var extendedType = '';`
`38`	`38`	`if (this._unsigned) {`
`39`		`- type += ' unsigned';`
	`39`	`+ extendedType += ' unsigned';`
`40`	`40`	`}`
`41`	`41`	`if (this._zerofill) {`
`42`		`- type += ' zerofill';`
	`42`	`+ extendedType += ' zerofill';`
`43`	`43`	`}`
`44`	`44`	`if (this._autoIncrement) {`
`45`		`- type += ' AUTO_INCREMENT';`
	`45`	`+ extendedType += ' AUTO_INCREMENT';`
`46`	`46`	`}`
`47`		`- return type;`
	`47`	`+ return extendedType;`
`48`	`48`	`}`
`49`	`49`	`}`
`50`	`50`
Original file line number	Diff line number	Diff line change
`@@ -48,15 +48,15 @@ class TextColumnDefinition extends ColumnDefinition {`
`48`	`48`	`return true;`
`49`	`49`	`}`
`50`	`50`
`51`		`- __getType() {`
`52`		`- var type = this.$type;`
	`51`	`+ __getExtendedType() {`
	`52`	`+ var extendedType = '';`
`53`	`53`	`if (this._charset) {`
`54`		`- type += ' CHARACTER SET ' + this._charset;`
	`54`	`+ extendedType += ' CHARACTER SET ' + this._charset;`
`55`	`55`	`}`
`56`	`56`	`if (this._collate) {`
`57`		`- type += ' COLLATE ' + this._collate;`
	`57`	`+ extendedType += ' COLLATE ' + this._collate;`
`58`	`58`	`}`
`59`		`- return type;`
	`59`	`+ return extendedType;`
`60`	`60`	`}`
`61`	`61`	`}`
`62`	`62`
Original file line number	Diff line number	Diff line change
`@@ -22,12 +22,8 @@ class UpdatableTimeColumnDefinition extends ColumnDefinition {`
`22`	`22`	`this._onUpdateCurrentTimestamp === columnDefinition._onUpdateCurrentTimestamp;`
`23`	`23`	`}`
`24`	`24`
`25`		`- __getType() {`
`26`		`- var type = this.$type;`
`27`		`- if (this._onUpdateCurrentTimestamp) {`
`28`		`- type += ' ON UPDATE CURRENT_TIMESTAMP';`
`29`		`- }`
`30`		`- return type;`
	`25`	`+ __getExtendedType() {`
	`26`	`+ return this._onUpdateCurrentTimestamp ? ' ON UPDATE CURRENT_TIMESTAMP' : '';`
`31`	`27`	`}`
`32`	`28`	`}`
`33`	`29`