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

defaultRaw current_timestamp(3) not taken into account in generated migration #4782

Closed
kalu06 opened this issue Oct 4, 2023 · 5 comments
Closed
Labels
bug Something isn't working

Comments

@kalu06
Copy link

kalu06 commented Oct 4, 2023

Describe the bug
When specifying defaultRaw: 'CURRENT_TIMESTAMP(3)' on a Date property on an entity, the generated migration will contain default current_timestamp(0)

To Reproduce
Steps to reproduce the behavior:

  1. Use the mysql type in mikro-orm.config.ts
  2. Create an entity that contains a Date property like this:
@Property({
  columnType: 'timestamp(3)',
  defaultRaw: 'CURRENT_TIMESTAMP(3)',
 })
updatedAt!: Date;
  1. Generate a migration using npx mikro-orm migration:create

Expected behavior
I would expect the migration to contain default current_timestamp(3) as specified in the entity defaultRaw.

Versions

Dependency Version
node 18.6.0
typescript 5.2.2
mikro-orm 5.8.3
@B4nan
Copy link
Member

B4nan commented Oct 4, 2023

Looks like an issue with defaultRaw, as with default it behaves correctly (but you can't use that for SQL functions, so won't help with this).

@B4nan B4nan added the bug Something isn't working label Oct 4, 2023
@kalu06
Copy link
Author

kalu06 commented Oct 4, 2023

Digging in the source code, I think I found the root cause in https://github.com/mikro-orm/mikro-orm/blob/master/packages/knex/src/schema/SchemaHelper.ts#L251.
It seems that knex uses the length prop to replace the precision of the current_timestamp from defaultRaw.

Changing my example above with the following seems to work:

@Property({
  columnType: 'timestamp(3)',
  defaultRaw: 'CURRENT_TIMESTAMP(3)',
  length: 3,
 })
updatedAt!: Date;

@B4nan
Copy link
Member

B4nan commented Oct 4, 2023

Hmm interesting, I'd say the problem is that we shouldn't touch the user-provided defaultRaw, just like we never touch the columnType option.

@B4nan
Copy link
Member

B4nan commented Oct 4, 2023

Right, the main issue here is the inference of the length for datetime columns were not implemented, we just used the platform default. That's why providing length explicitly helps.

https://github.com/mikro-orm/mikro-orm/blob/master/packages/knex/src/schema/DatabaseTable.ts#L85

@B4nan B4nan closed this as completed in cbc0c50 Oct 4, 2023
@kalu06
Copy link
Author

kalu06 commented Oct 5, 2023

@B4nan thanks for fixing this so quickly 👍

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

2 participants