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

Enum that contains functions is not accordingly translated to SQL #7651

Closed
3 of 21 tasks
dariosn85 opened this issue May 13, 2021 · 0 comments · Fixed by #7653
Closed
3 of 21 tasks

Enum that contains functions is not accordingly translated to SQL #7651

dariosn85 opened this issue May 13, 2021 · 0 comments · Fixed by #7653

Comments

@dariosn85
Copy link
Contributor

dariosn85 commented May 13, 2021

Issue Description

Typescript provides concept of declaration merging. From the documentation: “declaration merging” means that the compiler merges two separate declarations declared with the same name into a single definition.

This technique is often used to provide various utility functions in enums as we are used to from other languages (like Java and similar).

For example if we have following enum:

export enum Day {
    SUNDAY = 'sunday',
    MONDAY = 'monday',
    TUESDAY = 'tuesday',
    WEDNESDAY = 'wednesday',
    THURSDAY = 'thursday',
    FRIDAY = 'friday',
    SATURDAY = 'saturday'
}

and then we define namespace with same name as enum and with utility function from():

export namespace Day {
    export function from(day: string): Day {
        if (day === 'sun' || day === 'su' || day === 'sunday') {
            return Day.SUNDAY;
        }
        
        // ... rest of the code

        throw new Error(`Unknown enum ${day}`);
    }
}

this two declarations are merged. In most cases we access enum values like this:

const day = Day.MODAY;

but since we also defined from() in namespace with same name as enum is, we can reference it by referencing enum:

const day = Day.from('sun');

If we then for example define following entity:

@Entity({ name: 'day_enum_test' })
export class DayEnumTestEntity {

    @PrimaryColumn()
    id: string;

    @Column( { type: 'enum', enum: Day, default: Day.SUNDAY })
    day: Day;

}

Issue is that in such case Typeorm does not create SQL statements properly when we e.g. generate a migration.

Expected Behavior

In case when enum is created in the database (example is for postgresql) we expect that only actual enum values are provided without enum functions.

See: AS ENUM('sunday', 'monday', ... it contains only 'sunday', 'monday', 'tuesday', 'wednesday', 'thursday', 'friday', 'saturday'.

export class test1620891256583 implements MigrationInterface {
    name = 'test1620891256583'

    public async up(queryRunner: QueryRunner): Promise<void> {
        await queryRunner.query(`CREATE TYPE "testDatabaseSchema"."day_enum_test_day_enum" AS ENUM('sunday', 'monday', 'tuesday', 'wednesday', 'thursday', 'friday', 'saturday')`);
        await queryRunner.query(`CREATE TABLE "testDatabaseSchema"."day_enum_test" ("id" character varying NOT NULL, "day" "testDatabaseSchema"."day_enum_test_day_enum" NOT NULL DEFAULT 'sunday', CONSTRAINT "PK_599b919752788408ef71afd73dd" PRIMARY KEY ("id"))`);
    }

    public async down(queryRunner: QueryRunner): Promise<void> {
        await queryRunner.query(`DROP TABLE "testDatabaseSchema"."day_enum_test"`);
        await queryRunner.query(`DROP TYPE "testDatabaseSchema"."day_enum_test_day_enum"`);
    }

}

Actual Behavior

Unfortunately in our case because function is part of enum, it is included in generated SQL.

See: AS ENUM('sunday', 'monday', ... it contains also from() implemetation (function body was converted to string). This is causing later in some cases syntax error on the DB side while running migration or enum type in the DB contains what is should not.

export class test1620891209562 implements MigrationInterface {
    name = 'test1620891209562'

    public async up(queryRunner: QueryRunner): Promise<void> {
        await queryRunner.query(`CREATE TYPE "testDatabaseSchema"."day_enum_test_day_enum" AS ENUM('sunday', 'monday', 'tuesday', 'wednesday', 'thursday', 'friday', 'saturday', 'function from(day) {
        if (day === ''sun' || day === 'su' || day === 'sunday') {
            return Day.SUNDAY;
        }
        throw new Error(\`Unknown enum ${day}\`);
    }')`);
        await queryRunner.query(`CREATE TABLE "testDatabaseSchema"."day_enum_test" ("id" character varying NOT NULL, "day" "testDatabaseSchema"."day_enum_test_day_enum" NOT NULL DEFAULT 'sunday', CONSTRAINT "PK_599b919752788408ef71afd73dd" PRIMARY KEY ("id"))`);
    }

    public async down(queryRunner: QueryRunner): Promise<void> {
        await queryRunner.query(`DROP TABLE "testDatabaseSchema"."day_enum_test"`);
        await queryRunner.query(`DROP TYPE "testDatabaseSchema"."day_enum_test_day_enum"`);
    }

}

Steps to Reproduce

  1. Create day.ts and copy into it enum and namespace code from above.
  2. Create day-enum-test.entity.ts and copy into it entity code from above.
  3. Make sure you have ormconfig setup for postgres.
  4. Run migration typeorm migration:generate --name test.
  5. Check content of generated {TIMESTAMP}-test.ts file.

My Environment

Dependency Version
Node.js version v12.20.2
Typescript version v4.2.3
TypeORM version 0.2.32

Additional Context

Relevant Database Driver(s)

  • aurora-data-api
  • aurora-data-api-pg
  • better-sqlite3
  • cockroachdb
  • cordova
  • expo
  • mongodb
  • mysql
  • nativescript
  • oracle
  • postgres
  • react-native
  • sap
  • sqlite
  • sqlite-abstract
  • sqljs
  • sqlserver

Are you willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time, and I know how to start.
  • Yes, I have the time, but I don't know how to start. I would need guidance.
  • No, I don't have the time, although I believe I could do it if I had the time...
  • No, I don't have the time and I wouldn't even know how to start.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant