fix(schema): improve diffing of default values for strings and dates

Closes #2385

packages/core/src/types/DateTimeType.ts

@@ -5,7 +5,7 @@ import type { EntityProperty } from '../typings';
 export class DateTimeType extends Type<Date, string> {
 
   getColumnType(prop: EntityProperty, platform: Platform): string {
-    return platform.getDateTimeTypeDeclarationSQL({ length: prop.length });
+    return platform.getDateTimeTypeDeclarationSQL({ length: prop.length ?? 0 });
   }
 
   compareAsType(): string {

packages/knex/src/schema/SchemaComparator.ts

@@ -1,5 +1,5 @@
 import type { Dictionary, EntityProperty } from '@mikro-orm/core';
-import { BooleanType } from '@mikro-orm/core';
+import { BooleanType, DateTimeType } from '@mikro-orm/core';
 import type { Column, ForeignKey, Index, SchemaDifference, TableDifference } from '../typings';
 import type { DatabaseSchema } from './DatabaseSchema';
 import type { DatabaseTable } from './DatabaseTable';
@@ -440,6 +440,14 @@ export class SchemaComparator {
       return defaultValueFrom === defaultValueTo;
     }
 
+    if (to.mappedType instanceof DateTimeType && from.default && to.default) {
+      // normalize now/current_timestamp defaults, also remove `()` from the end of default expression
+      const defaultValueFrom = from.default.toLowerCase().replace('current_timestamp', 'now').replace(/\(\)$/, '');
+      const defaultValueTo = to.default.toLowerCase().replace('current_timestamp', 'now').replace(/\(\)$/, '');
+
+      return defaultValueFrom === defaultValueTo;
+    }
+
     if (from.default && to.default) {
       return from.default.toString().toLowerCase() === to.default.toString().toLowerCase();
     }

packages/mariadb/src/MariaDbDriver.ts

@@ -1,11 +1,12 @@
-import type { AnyEntity, Configuration, EntityDictionary, NativeInsertUpdateManyOptions, QueryResult, Transaction } from '@mikro-orm/core';
-import { AbstractSqlDriver, MySqlPlatform } from '@mikro-orm/mysql-base';
+import type { AnyEntity, Configuration, EntityDictionary, NativeInsertUpdateManyOptions, QueryResult } from '@mikro-orm/core';
+import { AbstractSqlDriver } from '@mikro-orm/mysql-base';
 import { MariaDbConnection } from './MariaDbConnection';
+import { MariaDbPlatform } from './MariaDbPlatform';
 
 export class MariaDbDriver extends AbstractSqlDriver<MariaDbConnection> {
 
   constructor(config: Configuration) {
-    super(config, new MySqlPlatform(), MariaDbConnection, ['knex', 'mariadb']);
+    super(config, new MariaDbPlatform(), MariaDbConnection, ['knex', 'mariadb']);
   }
 
   async nativeInsertMany<T extends AnyEntity<T>>(entityName: string, data: EntityDictionary<T>[], options: NativeInsertUpdateManyOptions<T> = {}): Promise<QueryResult<T>> {

packages/mariadb/src/MariaDbPlatform.ts

@@ -0,0 +1,8 @@
+import { MySqlPlatform } from '@mikro-orm/mysql-base';
+import { MariaDbSchemaHelper } from './MariaDbSchemaHelper';
+
+export class MariaDbPlatform extends MySqlPlatform {
+
+  protected readonly schemaHelper: MariaDbSchemaHelper = new MariaDbSchemaHelper(this);
+
+}

packages/mariadb/src/MariaDbSchemaHelper.ts

@@ -0,0 +1,10 @@
+import type { Type } from '@mikro-orm/core';
+import { MySqlSchemaHelper } from '@mikro-orm/mysql-base';
+
+export class MariaDbSchemaHelper extends MySqlSchemaHelper {
+
+  protected wrap(val: string | undefined, _type: Type<unknown>): string | undefined {
+    return val;
+  }
+
+}

packages/mariadb/src/index.ts

@@ -1,3 +1,5 @@
 export * from '@mikro-orm/mysql-base';
 export * from './MariaDbConnection';
+export * from './MariaDbSchemaHelper';
+export * from './MariaDbPlatform';
 export * from './MariaDbDriver';

packages/mysql-base/src/MySqlSchemaHelper.ts

@@ -1,6 +1,7 @@
 import type { AbstractSqlConnection, Column, Index, Knex, TableDifference } from '@mikro-orm/knex';
 import { SchemaHelper } from '@mikro-orm/knex';
-import type { Dictionary } from '@mikro-orm/core';
+import type { Dictionary , Type } from '@mikro-orm/core';
+import { StringType, TextType } from '@mikro-orm/core';
 
 export class MySqlSchemaHelper extends SchemaHelper {
 
@@ -115,17 +116,19 @@ export class MySqlSchemaHelper extends SchemaHelper {
       ifnull(datetime_precision, character_maximum_length) length
       from information_schema.columns where table_schema = database() and table_name = '${tableName}'`;
     const columns = await connection.execute<any[]>(sql);
+    const str = (val: string | number | undefined) => val != null ? '' + val : val;
 
     return columns.map(col => {
       const platform = connection.getPlatform();
       const mappedType = platform.getMappedType(col.column_type);
+      const defaultValue = str(this.normalizeDefaultValue(col.column_default, col.length));
       return ({
         name: col.column_name,
         type: platform.isNumericColumn(mappedType) ? col.column_type.replace(/ unsigned$/, '').replace(/\(\d+\)$/, '') : col.column_type,
         mappedType,
         unsigned: col.column_type.endsWith(' unsigned'),
         length: col.length,
-        default: col.column_default,
+        default: this.wrap(defaultValue, mappedType),
         nullable: col.is_nullable === 'YES',
         primary: col.column_key === 'PRI',
         unique: col.column_key === 'UNI',
@@ -153,4 +156,8 @@ export class MySqlSchemaHelper extends SchemaHelper {
     return super.normalizeDefaultValue(defaultValue, length, MySqlSchemaHelper.DEFAULT_VALUES);
   }
 
+  protected wrap(val: string | undefined, type: Type<unknown>): string | undefined {
+    return typeof val === 'string' && val.length > 0 && (type instanceof StringType || type instanceof TextType) ? this.platform.quoteValue(val) : val;
+  }
+
 }

packages/postgresql/src/PostgreSqlPlatform.ts

@@ -31,8 +31,8 @@ export class PostgreSqlPlatform extends AbstractSqlPlatform {
     return `current_timestamp(${length})`;
   }
 
-  getDateTimeTypeDeclarationSQL(column: { length?: number }): string {
-    return 'timestamptz' + (column.length != null ? `(${column.length})` : '');
+  getDateTimeTypeDeclarationSQL(column: { length: number }): string {
+    return `timestamptz(${column.length})`;
   }
 
   getTimeTypeDeclarationSQL(): string {

packages/sqlite/src/SqlitePlatform.ts

@@ -19,8 +19,8 @@ export class SqlitePlatform extends AbstractSqlPlatform {
     return super.getCurrentTimestampSQL(0);
   }
 
-  getDateTimeTypeDeclarationSQL(column: { length?: number }): string {
-    return super.getDateTimeTypeDeclarationSQL({ length: 0 });
+  getDateTimeTypeDeclarationSQL(column: { length: number }): string {
+    return 'datetime';
   }
 
   getEnumTypeDeclarationSQL(column: { items?: unknown[]; fieldNames: string[]; length?: number; unsigned?: boolean; autoincrement?: boolean }): string {

tests/features/entity-generator/__snapshots__/EntityGenerator.test.ts.snap

@@ -168,7 +168,7 @@ export class Book2 {
   @ManyToOne({ entity: () => Publisher2, onUpdateIntegrity: 'cascade', onDelete: 'cascade', nullable: true })
   publisher?: Publisher2;
 
-  @Property({ length: 255, nullable: true, defaultRaw: \`lol\` })
+  @Property({ length: 255, nullable: true, default: 'lol' })
   foo?: string;
 
 }

tests/features/migrations/__snapshots__/Migrator.test.ts.snap

@@ -372,7 +372,7 @@ class Migration20191013214813 extends Migration {
   }
 
   async down() {
-    this.addSql('alter table \`book2\` add \`foo\` varchar(255) null default lol;');
+    this.addSql('alter table \`book2\` add \`foo\` varchar(255) null default \\\\'lol\\\\';');
 
     this.addSql('alter table \`test2\` add \`foo___bar\` int unsigned null, add \`foo___baz\` int unsigned null;');
     this.addSql('alter table \`test2\` add constraint \`test2_foo___bar_foreign\` foreign key (\`foo___bar\`) references \`foo_bar2\` (\`id\`) on update cascade on delete set null;');
@@ -385,7 +385,7 @@ exports.Migration20191013214813 = Migration20191013214813;
 ",
   "diff": Object {
     "down": Array [
-      "alter table \`book2\` add \`foo\` varchar(255) null default lol;",
+      "alter table \`book2\` add \`foo\` varchar(255) null default 'lol';",
       "",
       "alter table \`test2\` add \`foo___bar\` int unsigned null, add \`foo___baz\` int unsigned null;",
       "alter table \`test2\` add constraint \`test2_foo___bar_foreign\` foreign key (\`foo___bar\`) references \`foo_bar2\` (\`id\`) on update cascade on delete set null;",
@@ -425,7 +425,7 @@ export class Migration20191013214813 extends Migration {
   }
 
   async down(): Promise<void> {
-    this.addSql('alter table \`book2\` add \`foo\` varchar(255) null default lol;');
+    this.addSql('alter table \`book2\` add \`foo\` varchar(255) null default \\\\'lol\\\\';');
 
     this.addSql('alter table \`test2\` add \`foo___bar\` int unsigned null, add \`foo___baz\` int unsigned null;');
     this.addSql('alter table \`test2\` add constraint \`test2_foo___bar_foreign\` foreign key (\`foo___bar\`) references \`foo_bar2\` (\`id\`) on update cascade on delete set null;');
@@ -437,7 +437,7 @@ export class Migration20191013214813 extends Migration {
 ",
   "diff": Object {
     "down": Array [
-      "alter table \`book2\` add \`foo\` varchar(255) null default lol;",
+      "alter table \`book2\` add \`foo\` varchar(255) null default 'lol';",
       "",
       "alter table \`test2\` add \`foo___bar\` int unsigned null, add \`foo___baz\` int unsigned null;",
       "alter table \`test2\` add constraint \`test2_foo___bar_foreign\` foreign key (\`foo___bar\`) references \`foo_bar2\` (\`id\`) on update cascade on delete set null;",
@@ -477,7 +477,7 @@ export class Migration20191013214813 extends Migration {
   }
 
   async down(): Promise<void> {
-    this.addSql('alter table \`book2\` add \`foo\` varchar(255) null default lol;');
+    this.addSql('alter table \`book2\` add \`foo\` varchar(255) null default \\\\'lol\\\\';');
 
     this.addSql('alter table \`test2\` add \`foo___bar\` int unsigned null, add \`foo___baz\` int unsigned null;');
     this.addSql('alter table \`test2\` add constraint \`test2_foo___bar_foreign\` foreign key (\`foo___bar\`) references \`foo_bar2\` (\`id\`) on update cascade on delete set null;');
@@ -489,7 +489,7 @@ export class Migration20191013214813 extends Migration {
 ",
   "diff": Object {
     "down": Array [
-      "alter table \`book2\` add \`foo\` varchar(255) null default lol;",
+      "alter table \`book2\` add \`foo\` varchar(255) null default 'lol';",
       "",
       "alter table \`test2\` add \`foo___bar\` int unsigned null, add \`foo___baz\` int unsigned null;",
       "alter table \`test2\` add constraint \`test2_foo___bar_foreign\` foreign key (\`foo___bar\`) references \`foo_bar2\` (\`id\`) on update cascade on delete set null;",

tests/features/schema-generator/__snapshots__/diffing-default-values.test.ts.snap

@@ -0,0 +1,40 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`diffing default values (GH #2385) string defaults do not produce additional diffs [mariadb] 1`] = `
+"set names utf8mb4;
+set foreign_key_checks = 0;
+
+create table \`foo1\` (\`id\` int unsigned not null auto_increment primary key, \`bar0\` varchar(255) not null default 'test', \`bar1\` varchar(255) not null default 'test', \`bar2\` datetime not null default now(), \`bar3\` datetime(6) not null default now(6)) default character set utf8mb4 engine = InnoDB;
+
+set foreign_key_checks = 1;
+"
+`;
+
+exports[`diffing default values (GH #2385) string defaults do not produce additional diffs [mysql] 1`] = `
+"set names utf8mb4;
+set foreign_key_checks = 0;
+
+create table \`foo1\` (\`id\` int unsigned not null auto_increment primary key, \`bar0\` varchar(255) not null default 'test', \`bar1\` varchar(255) not null default 'test', \`bar2\` datetime not null default now(), \`bar3\` datetime(6) not null default now(6)) default character set utf8mb4 engine = InnoDB;
+
+set foreign_key_checks = 1;
+"
+`;
+
+exports[`diffing default values (GH #2385) string defaults do not produce additional diffs [postgres] 1`] = `
+"set names 'utf8';
+set session_replication_role = 'replica';
+
+create table \\"foo2\\" (\\"id\\" serial primary key, \\"bar0\\" varchar(255) not null default 'test', \\"bar1\\" varchar(255) not null default 'test', \\"bar2\\" timestamptz(0) not null default now(), \\"bar3\\" timestamptz(6) not null default now());
+
+set session_replication_role = 'origin';
+"
+`;
+
+exports[`diffing default values (GH #2385) string defaults do not produce additional diffs [sqlite] 1`] = `
+"pragma foreign_keys = off;
+
+create table \`foo3\` (\`id\` integer not null primary key autoincrement, \`bar0\` text not null default 'test', \`bar1\` text not null default 'test', \`bar2\` datetime not null default now);
+
+pragma foreign_keys = on;
+"
+`;

tests/features/schema-generator/diffing-default-values.test.ts

@@ -0,0 +1,104 @@
+import { Entity, MikroORM, PrimaryKey, Property } from '@mikro-orm/core';
+
+export class Foo {
+
+  @PrimaryKey()
+  id!: number;
+
+  @Property({ defaultRaw: "'test'" })
+  bar0!: string;
+
+  @Property({ default: 'test' })
+  bar1!: string;
+
+}
+
+@Entity()
+export class Foo1 extends Foo {
+
+  @Property({ defaultRaw: 'now()' })
+  bar2!: Date;
+
+  @Property({ defaultRaw: 'now(6)', length: 6 })
+  bar3!: Date;
+
+}
+
+@Entity()
+export class Foo2 extends Foo {
+
+  @Property({ defaultRaw: 'now()' })
+  bar2!: Date;
+
+  @Property({ defaultRaw: 'now()', length: 6 })
+  bar3!: Date;
+
+}
+
+@Entity()
+export class Foo3 extends Foo {
+
+  @Property({ defaultRaw: 'now' })
+  bar2!: Date;
+
+}
+
+describe('diffing default values (GH #2385)', () => {
+
+  test('string defaults do not produce additional diffs [mysql]', async () => {
+    const orm = await MikroORM.init({
+      entities: [Foo1],
+      dbName: 'mikro_orm_test_gh_2385',
+      type: 'mysql',
+      port: 3307,
+    });
+    await orm.getSchemaGenerator().ensureDatabase();
+    await orm.getSchemaGenerator().dropSchema();
+    await orm.getSchemaGenerator().createSchema();
+    expect(await orm.getSchemaGenerator().getCreateSchemaSQL()).toMatchSnapshot();
+    await expect(orm.getSchemaGenerator().getUpdateSchemaSQL({ wrap: false })).resolves.toBe('');
+    await orm.close();
+  });
+
+  test('string defaults do not produce additional diffs [mariadb]', async () => {
+    const orm = await MikroORM.init({
+      entities: [Foo1],
+      dbName: 'mikro_orm_test_gh_2385',
+      type: 'mariadb',
+      port: 3309,
+    });
+    await orm.getSchemaGenerator().ensureDatabase();
+    await orm.getSchemaGenerator().dropSchema();
+    await orm.getSchemaGenerator().createSchema();
+    expect(await orm.getSchemaGenerator().getCreateSchemaSQL()).toMatchSnapshot();
+    await expect(orm.getSchemaGenerator().getUpdateSchemaSQL({ wrap: false })).resolves.toBe('');
+    await orm.close();
+  });
+
+  test('string defaults do not produce additional diffs [postgres]', async () => {
+    const orm = await MikroORM.init({
+      entities: [Foo2],
+      dbName: 'mikro_orm_test_gh_2385',
+      type: 'postgresql',
+    });
+    await orm.getSchemaGenerator().ensureDatabase();
+    await orm.getSchemaGenerator().dropSchema();
+    await orm.getSchemaGenerator().createSchema();
+    expect(await orm.getSchemaGenerator().getCreateSchemaSQL()).toMatchSnapshot();
+    await expect(orm.getSchemaGenerator().getUpdateSchemaSQL({ wrap: false })).resolves.toBe('');
+    await orm.close();
+  });
+
+  test('string defaults do not produce additional diffs [sqlite]', async () => {
+    const orm = await MikroORM.init({
+      entities: [Foo3],
+      dbName: ':memory:',
+      type: 'sqlite',
+    });
+    await orm.getSchemaGenerator().createSchema();
+    expect(await orm.getSchemaGenerator().getCreateSchemaSQL()).toMatchSnapshot();
+    await expect(orm.getSchemaGenerator().getUpdateSchemaSQL({ wrap: false })).resolves.toBe('');
+    await orm.close();
+  });
+
+});

tests/issues/__snapshots__/GH529.test.ts.snap

@@ -8,7 +8,7 @@ create table \\"product\\" (\\"id\\" serial primary key, \\"name\\" varchar(255)
 
 create table \\"customer\\" (\\"id\\" serial primary key);
 
-create table \\"order\\" (\\"id\\" serial primary key, \\"customer_id\\" int not null, \\"paid\\" boolean not null, \\"shipped\\" boolean not null, \\"created\\" timestamptz not null);
+create table \\"order\\" (\\"id\\" serial primary key, \\"customer_id\\" int not null, \\"paid\\" boolean not null, \\"shipped\\" boolean not null, \\"created\\" timestamptz(0) not null);
 
 create table \\"order_item\\" (\\"order_id\\" int not null, \\"product_id\\" int not null, \\"amount\\" int not null, \\"offered_price\\" int not null);
 alter table \\"order_item\\" add constraint \\"order_item_pkey\\" primary key (\\"order_id\\", \\"product_id\\");