Multiple Versions allowed in migration file names

ScottPierce commented 2 years ago

SQLDelight Version

1.5.3

Operating System

OSX

Gradle Version

7.4.2

Kotlin Version

1.6.10

Dialect

MySQL

AGP Version

No response

Describe the Bug

When using SqlDelight with FlyWay, someone named one of the migrations V12__InitialFormV2.sqm. This caused an issue where fields were being mapped in an incorrect order and values that should have been for field A were instead applied to field B. The solution was to remove the V2 from the end of V12__InitialFormV2.sqm and instead make it V12__InitialForm.sqm.

Tracking this issue down took a good amount of time, and required hacky modification flyway's database table to fix.

It seems reasonable that SqlDelight would error when 2 versions are potentially found in the file name. I'm assuming that's why this bug occurred.

Stacktrace

No response

Gradle Build Script

No response

hfhbd commented 1 year ago

Related to https://github.com/cashapp/sqldelight/issues/3513: We should add a check for unique migration numbers

AlecKazakova commented 1 year ago

Yea I agree, I think we should probably just fail the build if we find a migration file that has multiple strings of integers in it

dbacinski commented 1 year ago

It is very hard to get what is going on, at least build should fail. Would be great if migration versioning of sqldelight would match with Flyway.

hfhbd commented 1 year ago

@dbacinski What do you mean by would match with flyway?

dbacinski commented 1 year ago

In our case filename was V302__Salesorder_3ds_transaction.sqm, Flyway resolved version correctly as 302, but SqlDelight did something different (not sure what) so that changes form this migration were not applied. So most probably version was resolved to 3023 and migrations were not in expected order. Renaming to V302__Salesorder_threeds_transaction.sqm fixed the problem. It took hours to figure it out 😢

cashapp / sqldelight