bun icon indicating copy to clipboard operation
bun copied to clipboard

Migration tool is doing lexicographical sort on timestamp part of the migration filenames

Open jeffreydwalter opened this issue 8 months ago • 7 comments

The migration code is doing a lexicographical sort on the timestamp part of the migration filename. That is technically incorrect and fails if one uses a non-timestamp prefix (i.e., I use ints 0_migrate.up.sql, 1_migrate.up.sql, etc.).

This is the problematic code: https://github.com/uptrace/bun/blob/master/migrate/migration.go#L292-L302

There is zero harm in changing that code to convert the value to an int64 and performing a numeric sort. Doing so works in both cases.

jeffreydwalter avatar Apr 17 '25 22:04 jeffreydwalter

PR: https://github.com/uptrace/bun/pull/1186

jeffreydwalter avatar Apr 17 '25 22:04 jeffreydwalter

That is technically incorrect and fails if one uses a non-timestamp prefix (i.e., I use ints 0_migrate.up.sql, 1_migrate.up.sql, etc.).

Bun makes no explicit promises about using any particular sorting algorithm, which is why the current solution cannot be "technically incorrect". There is no universal correctness for sorting database migration files, only conventions which different ORMs adopt.

Bun's convention is to use timestamp + optional comment; it is a common format also used in PrismaORM, TypeORM, Entity Framework and others.

bevzzz avatar Apr 21 '25 17:04 bevzzz

Alright, you want to play that game, whatever. The timestamps are represented as ints in bun's case. You don't think sorting them as strings is incorrect? I think that's stupid.

jeffreydwalter avatar Apr 21 '25 21:04 jeffreydwalter

Even if other libraries do it, the choice of using a timestamp is also pretty stupid and a pain in the ass to deal with. It shouldn't necessitate a tool to generate file names because they are such a hassle to write by hand.

jeffreydwalter avatar Apr 21 '25 21:04 jeffreydwalter

While we use auto-incrementing numbers, we format the output using fixed-width numbers with leading zeros

Such as:

  • 0000_migrate.up.sql
  • 0001_migrate.up.sql

j2gg0s avatar Apr 23 '25 06:04 j2gg0s

@j2gg0s that would still sort correctly with my PR.

jeffreydwalter avatar Apr 23 '25 20:04 jeffreydwalter

This issue has been automatically marked as stale because it has not had activity in the last 30 days. If there is no update within the next 7 days, this issue will be closed.

github-actions[bot] avatar May 24 '25 03:05 github-actions[bot]