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

[jeffreydwalter]

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]

PR: #1186

[bevzzz]

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.

[jeffreydwalter]

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]

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.

[j2gg0s]

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

[jeffreydwalter]

@j2gg0s that would still sort correctly with my PR.