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

Unique property causes issue when generating a migration on a schema scoped table #1215

Closed
mmcxii opened this issue Dec 16, 2020 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@mmcxii
Copy link

mmcxii commented Dec 16, 2020

Describe the bug
When generating a migration for a table that is a part of a specific schema (a la @Entity({ tableName: "schemaName.tableName" })), the migrator will generate an invalid SQL statement.

This is where I decorate a column:

@Property({
  unique: true,
})
columnName: string;

I also tried it like this and it produced the same result:

@Property()
@Unique()
columnName: string;

This is what is generated:

alter table "schemaName"."tableName"
add constraint "schemaName"."tableName_columnName_unique" unique ("columnName");

However when I remove the "schemaName". prefix from the constraint like this, the migration successfully ran as expected.

alter table "schemaName"."tableName"
add constraint "tableName_columnName_unique" unique ("columnName");

Stack trace

SyntaxErrorException: alter table "schemaName"."tableName" add constraint "schemaName"."tableName_columnName_unique" unique ("columnName"); - syntax error at or near "."
    at PostgreSqlExceptionConverter.convertException (/.../node_modules/@mikro-orm/postgresql/PostgreSqlExceptionConverter.js:30:24)
    at PostgreSqlDriver.convertException (/.../node_modules/@mikro-orm/core/drivers/DatabaseDriver.js:171:54)
    at /.../node_modules/@mikro-orm/core/drivers/DatabaseDriver.js:175:24
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at Function.runSerial (/.../node_modules/@mikro-orm/core/utils/Utils.js:458:22)
    at /.../node_modules/@mikro-orm/migrations/MigrationRunner.js:23:17

previous error: alter table "schemaName"."tableName" add constraint "schemaName"."tableName_columnName_unique" unique ("columnName"); - syntax error at or near "."
    at Parser.parseErrorMessage (/.../node_modules/pg-protocol/src/parser.ts:357:11)
    at Parser.handlePacket (.../node_modules/pg-protocol/src/parser.ts:186:21)
    at Parser.parse (/.../node_modules/pg-protocol/src/parser.ts:101:30)
    at Socket.<anonymous> (/.../node_modules/pg-protocol/src/

To Reproduce
Steps to reproduce the behavior:

  1. Create a table with a schema scoped table name and a unique constraint on a column.
  2. Create a migration to apply the table to the database.
  3. Inspect the resulting SQL command.

Expected behavior
The migrator was expected to produce a valid SQL statement.

** Additional Context **
Using with Nest.js, however I don't think that is impacting this behavior.

Versions

Dependency Version
node 14.2.0
typescript 4.1.2
mikro-orm 4.3.4
@mikro-orm/postgresql 4.3.4
@mikro-orm/nestjs 4.2.0
@mmcxii
Copy link
Author

mmcxii commented Dec 18, 2020

A follow up on this, I was tinkering around with the migration generated some more and noticed that if I edit the constraint from ... add constraint "schemaName"."tableName_columnName_unique" unique ("columnName"); to add constraint "schemaName.tableName_columnName_unique" unique ("columnName"); (note that I combine the string wrapping the schema name and the string wrapping the constraint name) then it fixes the issue and the migration will run as expected.

@B4nan
Copy link
Member

B4nan commented Dec 18, 2020

I don't think we need to prefix constraint names, they will exist inside the schema only. The fix should be just about not passing the schema name inside. Will need to check, I believe we are not explicitly setting the name anywhere and its knex doing it, but it is definitely possible to set it explicitly.

@B4nan B4nan added the bug Something isn't working label Dec 18, 2020
@B4nan
Copy link
Member

B4nan commented Dec 18, 2020

Wait a minute, I got confused, thought its a FK constraint. You can set the unique constraint name manually already:

@Property({
  unique: 'column_name_uniq',
})
columnName: string;

or

@Property()
@Unique({ name: 'column_name_uniq' })
columnName: string;

@mmcxii
Copy link
Author

mmcxii commented Dec 18, 2020

Looks like explicitly providing the unique constraint by name works!

This is the resulting migration:

@Entity({
  tableName: "schemaName.tableName",
})
export class TableName {
  // ... other columns

  @Property({
    unique: "column_name_unique",
  })
  columnName: string;
}
alter table "schemaName"."tableName"
add constraint "column_name_unique" unique ("column_name");

@B4nan B4nan closed this as completed in b62d9ec Dec 18, 2020
@antl3x
Copy link

antl3x commented Dec 30, 2020

@B4nan this does not solve the error scenario for primary key constraints. It keeps adding the schema.

I'm on version 4.3.5-dev.28

Example:

    this.addSql('alter table "IAM"."Account" add constraint "IAM"."Account_pkey" primary key ("id");');

@antl3x
Copy link

antl3x commented Mar 19, 2021

@B4nan updates on this?

@B4nan
Copy link
Member

B4nan commented Mar 19, 2021

currently in the middle of full rewrite for schema diffing, targetting v5 as it will probably require some breaking changes. so you will need to wait a bit more, if you will see the same with v5, open new issue with full reproduction.

edit: but open for PR if its not a big change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants