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

Postgres migrateUp sometimes failing if migration name has a "-" character #843

Open
DavesBorges opened this issue Jan 15, 2024 · 2 comments · May be fixed by #844
Open

Postgres migrateUp sometimes failing if migration name has a "-" character #843

DavesBorges opened this issue Jan 15, 2024 · 2 comments · May be fixed by #844
Labels
bug Something isn't working migrations Related to migrations postgres Related to PostgreSQL

Comments

@DavesBorges
Copy link
Contributor

Description

In versions of kysely < 0.27.2, if migration names have a "-" character (beyond the usual ISO date string), running migrateToLatest against a postgres database might fail even if it had previously succeeded.
The postgres ORDER BY ignores "-" character when ordering strings (while javascript sort method considers it when sorting strings) causing the order of executed migrations returned from database and the order of candidate migrations to not be the same.

Since 0.27.2 this issue is less likely to occur because when
querying for executed migrations the ORDER BY uses the timestamp and secondarily the name column to order executed migrations (introduced in #723).

But it can still occurr if the system where the migrations are being
runned is extremly fast or the system clock is unreliable causing the timestamps to be the same.
That would result in the ordering of migrations being the same as if only the name column were used
in the ORDER BY clause.

Example.

We have the following pending migrations:

  • 2024-01-01-create-table
  • 2024-01-01.2-update-table

After running migrator.migrateToLatest() We get the following values in the table (the timestamps are the same)

name Timestamp
2024-01-01-create-table 2024-01-01T00:00:00.000Z
2024-01-01.2-update-table 2024-01-01T00:00:00.000Z

When migratetoLatest() is called again without allowUnorderedMigrations set to true
it will fail in version 0.27.2 and prior versions because the index of the candidate
migrations and the executed migrations won't match

index candiate Migrations executed_migrations (as read from db)
0 2024-01-01-create-table 2024-01-01.2-update-table
1 2024-01-01.2-update-table 2024-01-01-create-table
DavesBorges added a commit to DavesBorges/kysely that referenced this issue Jan 15, 2024
@koskimas
Copy link
Member

koskimas commented Jan 15, 2024

timestamp in the migrations table doesn't come from the file name. It's the execution time of the migration.

You should really use full ISO date-time strings in your migration names. Not just the dates.

But we should sort the migrations using javascript as you suggested 👍. Databases sort things differently depending on the collation.

@DavesBorges
Copy link
Contributor Author

timestamp in the migrations table doesn't come from the file name. It's the execution time of the migration.

Yes. In that scenario they were the same because we assume that they run very fast one after the other (or the system clock was unrelibable) and so they endup being the same date in the timestamp column

DavesBorges added a commit to DavesBorges/kysely that referenced this issue Jan 15, 2024
DavesBorges added a commit to DavesBorges/kysely that referenced this issue Jan 17, 2024
…ns enabled

This is needed to test if migrating down works when
the up migrations were not executed in alphabetical order
@igalklebanov igalklebanov added bug Something isn't working postgres Related to PostgreSQL migrations Related to migrations labels Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working migrations Related to migrations postgres Related to PostgreSQL
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants