typeorm / typeorm

ORM for TypeScript and JavaScript. Supports MySQL, PostgreSQL, MariaDB, SQLite, MS SQL Server, Oracle, SAP Hana, WebSQL databases. Works in NodeJS, Browser, Ionic, Cordova and Electron platforms.
http://typeorm.io
MIT License
34.03k stars 6.27k forks source link

Generate migration #7287

Open vrg-success opened 3 years ago

vrg-success commented 3 years ago

Issue Description

In each generated migration I get sql command to alter column "avatarUri" to default value, but default value is already in database

Expected Behavior

Clean migration with really required sql commands

Actual Behavior

import {MigrationInterface, QueryRunner} from "typeorm";

export class test1610830605429 implements MigrationInterface {
    name = 'test1610830605429'

    public async up(queryRunner: QueryRunner): Promise<void> {
        await queryRunner.query(`COMMENT ON COLUMN "users"."avatarUri" IS NULL`);
        await queryRunner.query(`ALTER TABLE "users" ALTER COLUMN "avatarUri" SET DEFAULT null`);
    }

    public async down(queryRunner: QueryRunner): Promise<void> {
        await queryRunner.query(`ALTER TABLE "users" ALTER COLUMN "avatarUri" DROP DEFAULT`);
        await queryRunner.query(`COMMENT ON COLUMN "users"."avatarUri" IS NULL`);
    }

}

Steps to Reproduce

Users.ts

import { Entity, PrimaryGeneratedColumn, Column, Index } from 'typeorm';

export enum USER_ROLES {
  USER = 'USER',
  ADMIN = 'ADMIN',
}

export enum USER_TYPES {
  NONE = 'NONE',
  USER = 'USER',
  CREATOR = 'CREATOR',
}

@Entity({ name: 'users' })
export class UsersEntity {
  @Index()
  @PrimaryGeneratedColumn('increment')
  id: number;

  @Column({ unique: true, length: 70 })
  username: string;

  @Column({ type: 'enum', enum: USER_ROLES, default: USER_ROLES.USER })
  role: string;

  @Column({ type: 'enum', enum: USER_TYPES, default: USER_TYPES.NONE })
  type: string;

  @Column({ unique: true }) email: string;
  @Column() password: string;
  @Column({ default: null }) avatarUri: string;
}

ormconfig.js

module.exports = [
  {
    name: 'default',
    type: 'postgres',
    url: 'postgres://postgres:qqq111@localhost:5432/typeorm-test',
    entities: ['src/entities/**/*.ts'],
    migrations: ['migrations/**/*.ts'],
    synchronize: false,
  },
];

Run command ts-node-dev --respawn -r tsconfig-paths/register ./node_modules/typeorm/cli.js migration:generate -d ./migrations -n test

My Environment

Dependency Version
Operating System Windows 10 PRO
Node.js version v2.16.3
Typescript version v4.0.3
TypeORM version v0.2.28

Additional Context

vrg-success commented 3 years ago

@pleerock @tomspeak @matt-neal @imnotjames @daniel-lang I reproduced the bug in video https://www.youtube.com/watch?v=L94X3enbdOg&feature=youtu.be

hood commented 3 years ago

I have similar issues in a pretty big application i'm working on: generated migrations contain a huge number of unneeded queries which need to be pruned manually. This Is happening mostly with "unsigned" columns, and "date" columns with default values, which trigger some totally redundant UPDATE TABLE queries every time. I'm on mysql, but this is happening with some other MariaDB projects too.

vrg-success commented 3 years ago

I have similar issues in a pretty big application i'm working on: generated migrations contain a huge number of unneeded queries which need to be pruned manually. This Is happening mostly with "unsigned" columns, and "date" columns with default values.

Yep, I to have a big app and all generated migration need to be pruned manually with fully focus concentration

Nightbr commented 3 years ago

Hey guys,

I get the same issue with my timestampableEntity:

import { CreateDateColumn, UpdateDateColumn } from 'typeorm';
import { Field, GraphQLISODateTime, ObjectType } from '@nestjs/graphql';

@ObjectType()
export class TimestampableEntity {
  @Field(() => GraphQLISODateTime)
  @CreateDateColumn({
    type: 'timestamp without time zone',
    default: () => 'CURRENT_TIMESTAMP'
  })
  public createdAt: Date;

  @Field(() => GraphQLISODateTime)
  @UpdateDateColumn({
    type: 'timestamp without time zone',
    default: () => 'CURRENT_TIMESTAMP'
  })
  public updatedAt: Date;
}

And I get COMMENT ON COLUMN "user"."created_at" IS NULL in each generated migrations.

My workaround:

Set explicit comment on this field (createdAt & updatedAt in my case)

import { CreateDateColumn, UpdateDateColumn } from 'typeorm';
import { Field, GraphQLISODateTime, ObjectType } from '@nestjs/graphql';

@ObjectType()
export class TimestampableEntity {
  @Field(() => GraphQLISODateTime)
  @CreateDateColumn({
    type: 'timestamp without time zone',
    default: () => 'CURRENT_TIMESTAMP',
    comment: 'Creation date',
  })
  public createdAt: Date;

  @Field(() => GraphQLISODateTime)
  @UpdateDateColumn({
    type: 'timestamp without time zone',
    default: () => 'CURRENT_TIMESTAMP',
    comment: 'Latest update date',
  })
  public updatedAt: Date;
}

And now I generate my migration and it doesn't regenerate the COMMENT ON COLUMN "user"."created_at" IS NULL in all next migration :tada:

No changes in database schema were found - cannot generate a migration. To create a new empty migration use "typeorm migration:create" command

krazibit commented 3 years ago

This does bug exists for certain default values that are not generated on the db schema exactly as it's defined in the model. But @Nightbr 's workaround works in most cases if you set the default to a func that returns how it's actually stored in the db schema. For example on jsonb columns with default

@Column({type: 'jsonb', default: () => "'{}'"})

In your case, if you want the column nullable just set nullable:true in the column opts and the default would be null, so no need to specify again, else if you want the actulal string literal null in which case you just wrap with quotes

culli commented 3 years ago

This bug seems to be caused by (at least in the PostgresDriver) not comparing the default expression case correctly. https://github.com/typeorm/typeorm/blob/e06a4423c83ae78a771cc239ee1135e70c98c899/src/driver/postgres/PostgresDriver.ts#L881

It should probably lowercase the right side of the comparison, maybe by using the lowerDefaultValueIfNecessary() that is used on the left side of the comparison?

|| (!tableColumn.isGenerated && this.lowerDefaultValueIfNecessary(this.normalizeDefault(columnMetadata)) !== this.lowerDefaultValueIfNecessary(tableColumn.default))

EDIT: in my case, using a default on the column with CURRENT_TIMESTAMP is the real issue. Somewhere that gets changed to now() and then TypeORM is seeing the column as changed (because it's now() in the metadata pulled from the db vs CURRENT_TIMESTAMP in the metadata pulled from the Entity.) Changing to use now() in keeps it from generating the COMMENT ON COLUMN statements.

tl;dr replace

@Column(`timestamptz`, { default: () => `CURRENT_TIMESTAMP` })

with

@Column(`timestamptz`, { default: () => `now()` })
hood commented 3 years ago

The problem is also present on pretty much all of my foreign key columns.

Here's a redundant migration and its related entity:

await queryRunner.query("ALTER TABLE `users_subscriptions` DROP FOREIGN KEY `FK_1`");
await queryRunner.query("ALTER TABLE `users_subscriptions` DROP FOREIGN KEY `FK_2`");
await queryRunner.query("ALTER TABLE `users_subscriptions` CHANGE `expires_at` `expires_at` datetime NULL");
await queryRunner.query("ALTER TABLE `users_subscriptions` CHANGE `user_id` `user_id` int UNSIGNED NULL");
await queryRunner.query("ALTER TABLE `users_subscriptions` CHANGE `subscription_id` `subscription_id` int UNSIGNED NULL");
@Entity({ name: 'users_subscriptions' })
export class UserSubscription extends BaseEntity {
  @PrimaryGeneratedColumn({ unsigned: true })
  id: number;

  @ManyToOne(
    type => User,
    user => user.subscriptions,
    {
      cascade: true
    }
  )
  user: User;

  @ManyToOne(type => Subscription, { cascade: true })
  subscription: Subscription;

  @Column({ default: true })
  active: boolean;

  @Column({ default: () => 'CURRENT_TIMESTAMP()', nullable: false })
  createdAt: Date;

  @Column({ nullable: true })
  expiresAt: Date;

  @Column({ default: 0 })
  maxUsages: number;
}
Rupikz commented 3 years ago

Specify explicitly that it is not null. My case:

@PrimaryGeneratedColumn('uuid', { name: 'id' })
id: string;

This worked:

@PrimaryGeneratedColumn('uuid', { name: 'id' })
id!: string;
stelescuraul commented 3 years ago

I have the same issue when trying to add a default on a column. Problem: Assume the following column and default:

  @Column({
    default: () => `current_setting('rls.tenant_id')::uuid`,
    comment: 'setting the default tenantId',
    nullable: false,
  })
  tenantId: string

This would allow me to insert rows without specifying the tenantId but the default would take care of that. The values in the session variable rls.tenant_id will always be text. From postgres docs:

New value of parameter. Values can be specified as string constants, identifiers, numbers, or comma-separated lists of these, as appropriate for the particular parameter. DEFAULT can be written to specify resetting the parameter to its default value (that is, whatever value it would have had if no SET had been executed in the current session).

This works directly in postgres but now I have the issue that whenever I try to generate a migration, it will always try to drop and recreate this default. I narrowed down my problem to this line: https://github.com/typeorm/typeorm/blob/master/src/driver/postgres/PostgresQueryRunner.ts#L1762

tableColumn.default = dbColumn["column_default"].replace(/::[\w\s\[\]\"]+/g, "");

The table column default reported by postgres looks like this: (current_setting('rls.tenant_id'::text))::uuid, however when it gets added to the tableColumn.default as part of identifying differences, these type casts are removed. I am not sure why you are removing the typecasts but this is indeed my problem. Could someone shed some light on this ?

The resulting value is: (current_setting('rls.tenant_id')) The default column value in entity metadata is correctly reported (just the value of the above function) but as you could tell, they won't match.

(current_setting('rls.tenant_id')) vs current_setting('rls.tenant_id')::uuid. Please see the extra ( ) around the default. I can fix the parenthesis by adding them myself to the default, but I cannot create the default without type casting.

Workaround: Declare the default as (current_setting('rls.tenant_id')) -> notice the extra ( ) and to typecast. Generate the migration and within the migration apply the type casting manually. Then whenever I regenerate the migration, now the defaults will match and there are no diffs generated.

As you can probably tell this is quite the hack and not really convenient. Easy to miss and make mistakes. The simplest solution would be to remove the regex replace of :: but I'm not sure what implications that will have. Any ideas ? @imnotjames @pleerock

semiautomatixfu commented 2 years ago

Yup definitely typecasting is the majority of my pain points on Postgres generation. Any recommended solutions, other than hacking away?