Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better schema diffing for MySQL timestamps managed on database side #2386

Closed
franck-co opened this issue Nov 9, 2021 · 0 comments
Closed
Labels
enhancement New feature or request

Comments

@franck-co
Copy link

Is your feature request related to a problem? Please describe.

It is already possible to define timestamps that are managed on db side :

@Entity()
export class EntityWithTimestamps {
    @PrimaryKey()
    id!: number
    
    @Property({ columnType: 'timestamp', defaultRaw: `current_timestamp`, getter: true })
    createdAt!: Date;

    @Property({ columnType: 'timestamp', defaultRaw: `current_timestamp on update current_timestamp`, getter: true })
    updatedAt!: Date;
}

But when generating a migration with the database as a source (= with disabled snapshots), the columns appears again and again in every migration.
It happens because the diffing does not handle properly MySQL timestamps
(or maybe because it is just not how I am supposed to define the createdAt and updatedAt columns 😅)

How to reproduce

  • Create an entity containing the createdAt and updatedAt properties as described.
  • Create a migration
  • Run the migration
  • Create a migration again.
    It should be empty but it's not

Identified problems with the diffing

(Definition: The SchemaComparator compares column1 from db and column2 from entity metadata.)

  1. 'timestamp' doesn't exist in Platform.getMappedType()
    As a result, the db column it is mapped to UnknownType and therefore to varchar(0) because datetime_precision=0
    The metadata column is also mapped to UnknownType but to varchar(255) because length is not specified in the entity definition.
    => SchemaComparator.diffColumn() always finds out that the type has changed, making the createdAt column appears again and again in migrations.

This can easily fixed by adding the timestamp type in Platform.getMappedType() and map it to DateTimeType

      case 'timestamp':
      case 'datetime': return types_1.Type.getType(types_1.DateTimeType);
  1. Diffing with defaultRaw: 'current_timestamp on update current_timestamp' does not work because on update current_timestamp is not a default value, it is an extra.

=> SchemaComparator.diffColumn() compares the metadata column defaultRaw (current_timestamp on update current_timestamp) with the db column default that only contains current_timestamp. It finds out the default has changed and the updatedAt column appear again and again in migrations.

A solution would be to allow to specify the an extra or mysqlExtra option in @Property decorator and to add diffing for this new option.

    @Property({ columnType: 'timestamp', defaultRaw: 'current_timestamp', extra: 'on update current_timestamp', getter: true })
    updatedAt!: Date;

Alternatives you considered

It is also possible to get it working without defining a new option 'extra'. It's easy but not very clean.
The idea is to search for 'on update current_timestamp' in the mysql column extra and append to the db column default just before the diffing.

In MySqlSchemaHelper

    //new method getColumnDefaultForDiffing
    getColumnDefaultForDiffing(col: Column, mappedType: Type){
        // hack to make schema diffing work with @Property({defaultRaw: 'current_timestamp on update current_timestamp', ...})
        if((mappedType instanceof DateTimeType && col.extra && col.extra.toLowerCase().includes("on update current_timestamp") )){
            return `${col.column_default} on update current_timestamp`
        }
        else{
            return col.column_default
        }
    }

  async getColumns(connection: AbstractSqlConnection, tableName: string, schemaName?: string): Promise<Column[]> {
    const sql = `select column_name as column_name,
      column_default as column_default,
      column_comment as column_comment,
      is_nullable as is_nullable,
      data_type as data_type,
      column_type as column_type,
      column_key as column_key,
      extra as extra,
      numeric_precision as numeric_precision,
      numeric_scale as numeric_scale,
      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);

    return columns.map(col => {
      const platform = connection.getPlatform();
      const mappedType = platform.getMappedType(col.column_type); //usage here
      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,
        nullable: col.is_nullable === 'YES',
        primary: col.column_key === 'PRI',
        unique: col.column_key === 'UNI',
        autoincrement: col.extra === 'auto_increment',
        precision: col.numeric_precision,
        scale: col.numeric_scale,
        comment: col.column_comment,
      });
    });
  }
@franck-co franck-co added the enhancement New feature or request label Nov 9, 2021
@B4nan B4nan closed this as completed in a224ec9 Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant