fix(schema): rework enum handling in schema generator

String enums are now discovered properly, and we diff the actual enum items to know whether something changed.
This also fixes postgres enum altering that produced invalid query.

Postgres dialect in knex is now monkey patched to have control over how column alters are working.
Now only necessary queries should be made (so no `drop null` followed by `set null`, etc).

Closes #397
Closes #432

lib/connections/PostgreSqlConnection.ts

@@ -1,10 +1,15 @@
 import { PgConnectionConfig } from 'knex';
 import { types, defaults } from 'pg';
 import { AbstractSqlConnection } from './AbstractSqlConnection';
+import { Dictionary } from '../typings';
+
+const TableCompiler_PG = require('knex/lib/dialects/postgres/schema/tablecompiler.js');
+const TableCompiler = require('knex/lib/schema/tablecompiler.js');
 
 export class PostgreSqlConnection extends AbstractSqlConnection {
 
   async connect(): Promise<void> {
+    this.patchKnex();
     this.client = this.createKnexClient('pg');
   }
 
@@ -40,4 +45,72 @@ export class PostgreSqlConnection extends AbstractSqlConnection {
     } as unknown as T;
   }
 
+  /**
+   * monkey patch knex' postgres dialect so it correctly handles column updates (especially enums)
+   */
+  private patchKnex(): void {
+    const that = this;
+
+    TableCompiler_PG.prototype.addColumns = function (this: typeof TableCompiler_PG, columns: Dictionary[], prefix: string, colCompilers: Dictionary[]) {
+      if (prefix !== this.alterColumnsPrefix) {
+        // base class implementation for normal add
+        return TableCompiler.prototype.addColumns.call(this, columns, prefix);
+      }
+
+      // alter columns
+      for (const col of colCompilers) {
+        that.addColumn.call(this, col, that);
+      }
+    };
+  }
+
+  private addColumn(this: typeof TableCompiler_PG, col: Dictionary, that: PostgreSqlConnection): void {
+    const quotedTableName = this.tableName();
+    const type = col.getColumnType();
+    const colName = this.client.wrapIdentifier(col.getColumnName(), col.columnBuilder.queryContext());
+    const constraintName = `${this.tableNameRaw}_${col.getColumnName()}_check`;
+    this.pushQuery({ sql: `alter table ${quotedTableName} drop constraint if exists "${constraintName}"`, bindings: [] });
+
+    if (col.type === 'enu') {
+      this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} type text using (${colName}::text)`, bindings: [] });
+      this.pushQuery({ sql: `alter table ${quotedTableName} add constraint "${constraintName}" ${type.replace(/^text /, '')}`, bindings: [] });
+    } else {
+      this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} type ${type} using (${colName}::${type})`, bindings: [] });
+    }
+
+    that.alterColumnDefault.call(this, col, colName);
+    that.alterColumnNullable.call(this, col, colName);
+  }
+
+  private alterColumnNullable(this: typeof TableCompiler_PG, col: Dictionary, colName: string): void {
+    const quotedTableName = this.tableName();
+    const nullable = col.modified.nullable;
+
+    if (!nullable) {
+      return;
+    }
+
+    if (nullable[0] === false) {
+      this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} set not null`, bindings: [] });
+    } else {
+      this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} drop not null`, bindings: [] });
+    }
+  }
+
+  private alterColumnDefault(this: typeof TableCompiler_PG, col: Dictionary, colName: string): void {
+    const quotedTableName = this.tableName();
+    const defaultTo = col.modified.defaultTo;
+
+    if (!defaultTo) {
+      return;
+    }
+
+    if (defaultTo[0] === null) {
+      this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} drop default`, bindings: [] });
+    } else {
+      const modifier = col.defaultTo.apply(col, defaultTo);
+      this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} set ${modifier}`, bindings: [] });
+    }
+  }
+
 }

lib/connections/SqliteConnection.ts

@@ -2,8 +2,6 @@ import { ensureDir, readFile } from 'fs-extra';
 import { dirname } from 'path';
 import { Config } from 'knex';
 
-const Bluebird = require('bluebird');
-
 import { AbstractSqlConnection } from './AbstractSqlConnection';
 
 export class SqliteConnection extends AbstractSqlConnection {
@@ -71,7 +69,7 @@ export class SqliteConnection extends AbstractSqlConnection {
     dialect.prototype._query = (connection: any, obj: any) => {
       const callMethod = this.getCallMethod(obj);
 
-      return new Bluebird((resolve: any, reject: any) => {
+      return new Promise((resolve: any, reject: any) => {
         /* istanbul ignore if */
         if (!connection || !connection[callMethod]) {
           return reject(new Error(`Error calling ${callMethod} on connection.`));

lib/schema/DatabaseSchema.ts

@@ -36,7 +36,8 @@ export class DatabaseSchema {
       const indexes = await helper.getIndexes(connection, t.table_name, t.schema_name);
       const pks = await helper.getPrimaryKeys(connection, indexes, t.table_name, t.schema_name);
       const fks = await helper.getForeignKeys(connection, t.table_name, t.schema_name);
-      table.init(cols, indexes, pks, fks);
+      const enums = await helper.getEnumDefinitions(connection, t.table_name, t.schema_name);
+      table.init(cols, indexes, pks, fks, enums);
     }
 
     return schema;

lib/schema/DatabaseTable.ts

@@ -35,7 +35,7 @@ export class DatabaseTable {
     }, {} as Dictionary<Index[]>);
   }
 
-  init(cols: Column[], indexes: Index[], pks: string[], fks: Dictionary<ForeignKey>): void {
+  init(cols: Column[], indexes: Index[], pks: string[], fks: Dictionary<ForeignKey>, enums: Dictionary<string[]>): void {
     this.indexes = indexes;
     this.foreignKeys = fks;
 
@@ -51,10 +51,11 @@ export class DatabaseTable {
       v.fk = fks[v.name];
       v.indexes = index.filter(i => !i.primary && !i.composite);
       v.defaultValue = v.defaultValue && v.defaultValue.toString().startsWith('nextval(') ? null : v.defaultValue;
+      v.enumItems = enums[v.name] || [];
       o[v.name] = v;
 
       return o;
-    }, {} as any);
+    }, {} as Dictionary<Column>);
   }
 
   getEntityDeclaration(namingStrategy: NamingStrategy, schemaHelper: SchemaHelper): EntityMetadata {
@@ -180,6 +181,7 @@ export interface Column {
   nullable: boolean;
   maxLength: number;
   defaultValue: string | null;
+  enumItems: string[];
 }
 
 export interface ForeignKey {

lib/schema/MySqlSchemaHelper.ts

@@ -1,6 +1,6 @@
 import { CreateTableBuilder } from 'knex';
 import { IsSame, SchemaHelper } from './SchemaHelper';
-import { EntityProperty } from '../typings';
+import { Dictionary, EntityProperty } from '../typings';
 import { AbstractSqlConnection } from '../connections/AbstractSqlConnection';
 import { Column, Index } from './DatabaseTable';
 
@@ -13,9 +13,9 @@ export class MySqlSchemaHelper extends SchemaHelper {
     double: ['double'],
     tinyint: ['tinyint'],
     smallint: ['smallint'],
-    string: ['varchar(?)', 'varchar', 'text', 'enum', 'bigint'],
     Date: ['datetime(?)', 'timestamp(?)', 'datetime', 'timestamp'],
     date: ['datetime(?)', 'timestamp(?)', 'datetime', 'timestamp'],
+    string: ['varchar(?)', 'varchar', 'text', 'bigint', 'enum'],
     text: ['text'],
     object: ['json'],
     json: ['json'],
@@ -71,6 +71,17 @@ export class MySqlSchemaHelper extends SchemaHelper {
       + `where k.table_name = '${tableName}' and k.table_schema = database() and c.constraint_schema = database() and k.referenced_column_name is not null`;
   }
 
+  async getEnumDefinitions(connection: AbstractSqlConnection, tableName: string, schemaName?: string): Promise<Dictionary> {
+    const sql =  `select column_name as column_name, column_type as column_type from information_schema.columns
+      where data_type = 'enum' and table_name = '${tableName}'`;
+    const enums = await connection.execute<any[]>(sql);
+
+    return enums.reduce((o, item) => {
+      o[item.column_name] = item.column_type.match(/enum\((.*)\)/)[1].split(',').map((item: string) => item.match(/'(.*)'/)![1]);
+      return o;
+    }, {} as Dictionary<string>);
+  }
+
   async getColumns(connection: AbstractSqlConnection, tableName: string, schemaName?: string): Promise<any[]> {
     const sql = `select column_name as column_name, column_default as column_default, is_nullable as is_nullable, data_type as data_type, column_key as column_key, ifnull(datetime_precision, character_maximum_length) length
       from information_schema.columns where table_schema = database() and table_name = '${tableName}'`;

lib/schema/PostgreSqlSchemaHelper.ts

@@ -1,5 +1,5 @@
 import { IsSame, SchemaHelper } from './SchemaHelper';
-import { EntityProperty } from '../typings';
+import { Dictionary, EntityProperty } from '../typings';
 import { AbstractSqlConnection } from '../connections/AbstractSqlConnection';
 import { Column, Index } from './DatabaseTable';
 
@@ -8,18 +8,18 @@ export class PostgreSqlSchemaHelper extends SchemaHelper {
   static readonly TYPES = {
     boolean: ['bool', 'boolean'],
     number: ['int4', 'integer', 'int2', 'int', 'float', 'float8', 'double', 'double precision', 'bigint', 'smallint', 'decimal', 'numeric', 'real'],
+    string: ['varchar(?)', 'character varying', 'text', 'character', 'char', 'uuid', 'bigint', 'int8', 'enum'],
     float: ['float'],
     double: ['double precision', 'float8'],
     tinyint: ['int2'],
     smallint: ['int2'],
-    string: ['varchar(?)', 'character varying', 'text', 'character', 'char', 'uuid', 'enum', 'bigint', 'int8'],
+    text: ['text'],
     Date: ['timestamptz(?)', 'timestamp(?)', 'datetime(?)', 'timestamp with time zone', 'timestamp without time zone', 'datetimetz', 'time', 'date', 'timetz', 'datetz'],
     date: ['timestamptz(?)', 'timestamp(?)', 'datetime(?)', 'timestamp with time zone', 'timestamp without time zone', 'datetimetz', 'time', 'date', 'timetz', 'datetz'],
-    text: ['text'],
     object: ['json'],
     json: ['json'],
     uuid: ['uuid'],
-    enum: ['enum'],
+    enum: ['text'], // enums are implemented as text columns with check constraints
   };
 
   static readonly DEFAULT_TYPE_LENGTHS = {
@@ -111,6 +111,24 @@ export class PostgreSqlSchemaHelper extends SchemaHelper {
       order by kcu.table_schema, kcu.table_name, kcu.ordinal_position`;
   }
 
+  async getEnumDefinitions(connection: AbstractSqlConnection, tableName: string, schemaName?: string): Promise<Dictionary> {
+    const sql = `select conrelid::regclass as table_from, conname, pg_get_constraintdef(c.oid) as enum_def
+      from pg_constraint c join pg_namespace n on n.oid = c.connamespace
+      where contype = 'c' and conrelid = '${schemaName}.${tableName}'::regclass order by contype`;
+    const enums = await connection.execute<any[]>(sql);
+
+    return enums.reduce((o, item) => {
+      // check constraints are defined as one of:
+      // `CHECK ((type = ANY (ARRAY['local'::text, 'global'::text])))`
+      // `CHECK (((enum_test)::text = ANY ((ARRAY['a'::character varying, 'b'::character varying, 'c'::character varying])::text[])))`
+      const m1 = item.enum_def.match(/check \(\(\((\w+)\)::/i) || item.enum_def.match(/check \(\((\w+) = any/i);
+      const m2 = item.enum_def.match(/\(array\[(.*)]\)/i);
+      o[m1[1]] = m2[1].split(',').map((item: string) => item.trim().match(/^'(.*)'/)![1]);
+
+      return o;
+    }, {} as Dictionary<string>);
+  }
+
   normalizeDefaultValue(defaultValue: string, length: number) {
     if (!defaultValue) {
       return defaultValue;

lib/schema/SchemaGenerator.ts

@@ -349,19 +349,21 @@ export class SchemaGenerator {
 
   private configureColumn<T>(meta: EntityMetadata<T>, prop: EntityProperty<T>, col: ColumnBuilder, columnName: string, pkProp = prop, alter?: IsSame) {
     const nullable = (alter && this.platform.requiresNullableForAlteringColumn()) || prop.nullable!;
+    const sameNullable = alter && 'sameNullable' in alter && alter.sameNullable;
     const indexed = 'index' in prop ? prop.index : (prop.reference !== ReferenceType.SCALAR && this.helper.indexForeignKeys());
     const index = (indexed || (prop.primary && meta.compositePK)) && !(alter && alter.sameIndex);
     const indexName = this.getIndexName(meta, prop, false, columnName);
     const uniqueName = this.getIndexName(meta, prop, true, columnName);
     const hasDefault = typeof prop.default !== 'undefined'; // support falsy default values like `0`, `false` or empty string
+    const sameDefault = alter && 'sameDefault' in alter ? alter.sameDefault : !hasDefault;
 
-    Utils.runIfNotEmpty(() => col.nullable(), nullable);
-    Utils.runIfNotEmpty(() => col.notNullable(), !nullable);
+    Utils.runIfNotEmpty(() => col.nullable(), !sameNullable && nullable);
+    Utils.runIfNotEmpty(() => col.notNullable(), !sameNullable && !nullable);
     Utils.runIfNotEmpty(() => col.primary(), prop.primary && !meta.compositePK);
     Utils.runIfNotEmpty(() => col.unsigned(), pkProp.unsigned);
     Utils.runIfNotEmpty(() => col.index(indexName), index);
     Utils.runIfNotEmpty(() => col.unique(uniqueName), prop.unique);
-    Utils.runIfNotEmpty(() => col.defaultTo(this.knex.raw('' + prop.default)), hasDefault);
+    Utils.runIfNotEmpty(() => col.defaultTo(hasDefault ? this.knex.raw('' + prop.default) : null), !sameDefault);
 
     return col;
   }

lib/schema/SchemaHelper.ts

@@ -38,12 +38,13 @@ export abstract class SchemaHelper {
 
   isSame(prop: EntityProperty, column: Column, idx = 0, types: Dictionary<string[]> = {}, defaultValues: Dictionary<string[]> = {}): IsSame {
     const sameTypes = this.hasSameType(prop.columnTypes[idx], column.type, types);
+    const sameEnums = this.hasSameEnumDefinition(prop, column);
     const sameNullable = column.nullable === !!prop.nullable;
     const sameDefault = this.hasSameDefaultValue(column, prop, defaultValues);
     const sameIndex = this.hasSameIndex(prop, column);
-    const all = sameTypes && sameNullable && sameDefault && sameIndex;
+    const all = sameTypes && sameNullable && sameDefault && sameIndex && sameEnums;
 
-    return { all, sameTypes, sameNullable, sameDefault, sameIndex };
+    return { all, sameTypes, sameEnums, sameNullable, sameDefault, sameIndex };
   }
 
   supportsSchemaConstraints(): boolean {
@@ -73,6 +74,10 @@ export abstract class SchemaHelper {
     return this.mapForeignKeys(fks);
   }
 
+  async getEnumDefinitions(connection: AbstractSqlConnection, tableName: string, schemaName?: string): Promise<Dictionary> {
+    return {};
+  }
+
   getListTablesSQL(): string {
     throw new Error('Not supported by given driver');
   }
@@ -228,6 +233,18 @@ export abstract class SchemaHelper {
     });
   }
 
+  private hasSameEnumDefinition(prop: EntityProperty, column: Column): boolean {
+    if (!prop.enum || !prop.items) {
+      return true;
+    }
+
+    if (prop.items.every(item => typeof item === 'number')) {
+      return true;
+    }
+
+    return Utils.equals(prop.items, column.enumItems);
+  }
+
 }
 
 export interface IsSame {
@@ -236,4 +253,5 @@ export interface IsSame {
   sameNullable?: boolean;
   sameDefault?: boolean;
   sameIndex?: boolean;
+  sameEnums?: boolean;
 }

package.json

@@ -96,7 +96,7 @@
     "fast-deep-equal": "^3.1.1",
     "fs-extra": "^8.1.0",
     "globby": "^10.0.0",
-    "knex": "^0.20.12",
+    "knex": "^0.20.13",
     "reflect-metadata": "^0.1.13",
     "ts-morph": "^4.3.3",
     "umzug": "^2.2.0",

tests/Migrator.test.ts

@@ -48,7 +48,6 @@ describe('Migrator', () => {
   });
 
   test('generate schema migration', async () => {
-    orm.em.getConnection().execute('drop table if exists new_table');
     const dateMock = jest.spyOn(Date.prototype, 'toISOString');
     dateMock.mockReturnValue('2019-10-13T21:48:13.382Z');
     const migrator = orm.getMigrator();