Add tests to guard against accidental modification of Flyway migrations

[aalbu]

Versioned Flyway migrations are intended to be applied only once. Consequently, they should be immutable. Flyway records a checksum of the migration after it is applied, to prevent accidental modification. Modifying a migration will cause Flyway to fail when the migration is applied.

We should have tests that prevent situations like that. Current tests start with an empty database and verify that migrations are properly applied. A more realistic scenario would be to run Flyway against a database that already had some migrations applied. For example, the database could be "primed" with the migrations present in the previous Trino release.

[kokosing]

How about a convention where migration file is versioned with the project version. So one cannot update the file of the version that was released.

Then we need tests that try to apply the new migration (from the current) version on non empty database with the previous version. Like test (or few tests) per version which are called in order to make sure that we can migrate from version to version.

For example

-- V369.sql
CREATE TABLE t (i int);

-- V370.sql
ALTER TABLE T add COLUMN j int;

Then tests (singlethreaded):

@Test
public void test369() {
  // - get previous version (in this case empty database)
  // - assert db is empty
  // - do migration to 369 add data
  // - do some assertions
}  

@Test
public void test370() {
  // - get previous version (in this case after version 369)
  // - assert that db is at 369 and it is non empty
  // - do migration to 370 add data
  // - do some assertions
}  

[aalbu]

The original concern was that migrations may be accidentally modified (e.g. reformatting). Such a test would not cover that scenario.

[kokosing]

modified (e.g. reformatting)

We can have additional test would assert checksums for flyway files.

[hashhar]

Simplest to me seems to be what @kokosing proposed now - we can have a test that verifies checksums of all migrations and also asserts that all files have a checksum present (to catch when new migrations are added).

It does provide a lesser guarantee though - it won't verify whether migrations will apply correctly on version upgrades but it does protect against the original concern.

For cross-version migration tests I think the only option would be a product test but that seems too heavy.

[findepi]

Simplest to me seems to be what @kokosing proposed now - we can have a test that verifies checksums of all migrations and also asserts that all files have a checksum present (to catch when new migrations are added).

That's laborous during development, when working on such a file.
But otherwise yes, it would work.

For cross-version migration tests I think the only option would be a product test but that seems too heavy.

is it? we already do this.

[aalbu]

I was thinking we could start the previous Trino version in a container, together with a database, and run the current migrations against the database (which would already contain the migrations from the previous version). Wouldn't have to be a product test.