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

@Property comment with PostgreSQL driver generates invalid SQL #4108

Closed
yeedle opened this issue Mar 8, 2023 · 3 comments · Fixed by #4123
Closed

@Property comment with PostgreSQL driver generates invalid SQL #4108

yeedle opened this issue Mar 8, 2023 · 3 comments · Fixed by #4123
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@yeedle
Copy link
Contributor

yeedle commented Mar 8, 2023

Describe the bug

When using comment in @Property annotations for a postgresql database, the generated SQL only uses the table name, and not the fully qualified schema name and table name (e.g. comment on "table_name"."column_name" is \'hello world\'; instead of comment on "schema_name"."table_name"."column_name" is \'hello world\';). This causes migrations to fail when the schema is not public

Stack trace
example of a migration when comment is used on an entity property.

MigrationError: Migration Migration20230307174746 (up) failed: Original error: comment on column "table_name"."column_name" is 'Hello world'; - relation "table_name" does not exist
    at /Users/yeedle/project/node_modules/umzug/src/umzug.ts:259:12
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Umzug.runCommand (/Users/yeedle/project/node_modules/umzug/src/umzug.ts:208:11)
    at async Migrator.runInTransaction (/Users/yeedle/project/node_modules/@mikro-orm/migrations/Migrator.js:299:21)
    at async PostgreSqlConnection.transactional (/Users/yeedle/project/node_modules/@mikro-orm/knex/AbstractSqlConnection.js:36:25)
    at async Function.handleFreshCommand (/Users/yeedle/project/node_modules/@mikro-orm/cli/commands/MigrationCommandFactory.js:153:9)
    at async Function.handleMigrationCommand (/Users/yeedle/project/node_modules/@mikro-orm/cli/commands/MigrationCommandFactory.js:88:17) {
  cause: TableNotFoundException: comment on column "table_name"."column_name" is 'Hello world'; - relation "table_name" does not exist

To Reproduce
Steps to reproduce the behavior:

  1. Add a comment to an entity @Property
  2. run migration:create
  3. run migration:up

Expected behavior
Generated SQL should include the table schema so that migrations succeed

Additional context
I'm happy to make a PR fixing this issue for Postgresql.

Versions

Dependency Version
node 16.14.2
typescript 1.22.15
mikro-orm 5.6.8
your-driver PostgreSQL
@B4nan
Copy link
Member

B4nan commented Mar 8, 2023

I guess the issue comes from this place:

this.helper.pushTableQuery(table, this.helper.getChangeColumnCommentSQL(tableName, column));

I think the same fix should be applied on the line that calls getAlterColumnAutoincrement too.

@B4nan B4nan added bug Something isn't working good first issue Good for newcomers labels Mar 8, 2023
@yeedle
Copy link
Contributor Author

yeedle commented Mar 9, 2023

Sounds about right. Do you mind if I open a PR with a fix for this (my approach would be to pass in tableSchema as an optional arg set to null by default, and use it in PostgreSqlSchemaHelper.ts for getChangeColumnSet and getAlterColumnAutoIncrement) ?

@B4nan
Copy link
Member

B4nan commented Mar 9, 2023

Go for it, PR welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
2 participants