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

[BUG] Objection IdColumn with type uuid generates the ID during migration, not during insertion #2947

Open
LonguCodes opened this issue Dec 25, 2024 · 2 comments
Assignees
Labels

Comments

@LonguCodes
Copy link

Describe the bug

Function createIdColumn in @tsed/objection generates uuid using crypto, but that means every new inserted value will have the same uuid.

Instead, the default value should point to some kind o uuid generation algorithm in the database, to have per-row uuid generation during insertion

To Reproduce

  1. Install @tsed/objection
  2. Create model with `@IdColumn('uuid')
  3. Create migration using createColumns
  4. Migrate

Expected behavior

The uuid generation should be per row, so the default should be set eg. uuid_v4_generate() in postgres

Code snippets

https://github.com/tsedio/tsed/blob/0e999d593dfd6340243cf43b6763e4d2b9f05fd5/packages/orm/objection/src/components/createIdColumn.ts#L19C59-L19C69

Repository URL example

https://github.com/tsedio/tsed/blob/0e999d593dfd6340243cf43b6763e4d2b9f05fd5/packages/orm/objection/src/components/createIdColumn.ts#L19C59-L19C69

OS

macOs

Node version

Node v22.12.0

Library version

v7.84.0

Additional context

No response

@Romakita
Copy link
Collaborator

@LonguCodes if you have precise idea of the fix, I suggest to make a PR.
The original owner of this package haven’t contributed on this package since a long time and I don’t use this package.

So let’s go to make a PR ;)
See you

@LonguCodes
Copy link
Author

I've looked into this further and seems like neither objection nor knex have a build-in way of creating a database-agnostic column with uuid generation as default.

I don't think implementation of database detection is in scope for a @tSed package.

My proposition would be to update the docs to remove the function usage in migration, as it also propagates potentially buggy patterns in migrations, besides the issue with uuids.
I'll prepare a PR with docs update in the near future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants