ebean-orm / ebean-migration

DB Migration runner (similar to Flyway) which can be used standalone or with Ebean (run migrations on EbeanServer start)
Apache License 2.0
9 stars 5 forks source link

Change the checksum computation to be before parameter replacement #91

Closed rbygrave closed 1 year ago

rbygrave commented 3 years ago

So, I have the plan that migration generation can generate idx<platform>.migrations files. These files contain the checksum and migration resource (this actually already exists in 12.11.0 in ebean).

The problem is that this checksum is based on the sql file resources as they exist on the file system. The checksum that ebean-migration uses is computed after the parameter expression replacement ( like ${...} ).

Ideally ebean-migration changes such that the checksum is computed before the parameter expression and in this way we can use idx<platform>.migrations files to:

That is, especially in CI/CD environment a lot of application starts happen with no actual migrations to run. ebean-migration does a pretty decent amount of work which gets more and more as the number of database migrations grows. We can use the idx<platform>.migrations files to make this case a lot faster and more efficient which will be pretty handy when there are lots of DB migrations and/or those migration files are large.

The challenge is to more the computation of checksum (which will change the checksum value for migrations that have property evaluation / replacement) and do this transparently without breaking peoples existing migrations.

My current plan is to use the 0 migration which has a checksum value of 0. Changing that to 1 to indicate that the new checksum is being used - when that 0 migration has a checksum value of 0 that would indicate that it uses the old checksum compute.

So we need to be careful here but we should get a good long term payoff in dealing with lots of migrations and or large migration files.

FYI @rPraml

rPraml commented 3 years ago

@rbygrave FYI:

It seems, that ebean 12 does already compute the checksum before parameter replacement, because all migrations with parameters has checksum errors now. (but this issue is still open and I did not find an issue/commit that's pointing out the change.)

This is not a big deal, for us, because only a few migrations are affected and we added as it could be easily fixed by specifying them in the ebean.migration.patchResetChecksumOn=1.234 property.

In other words: From our perspective, we can live with that incompatibility.

cheers Roland

rbygrave commented 2 years ago

It seems, that ebean 12 does already compute the checksum before parameter replacement, because all migrations with parameters has checksum errors now

That should not be the case no. I have not yet made that change, so I'm unsure why you saw checksum errors - odd?

nPraml commented 2 years ago

@rbygrave FYI: our problem was first of all with the special characters e.g. 'ü' in migration scripts. Maven in cmd and Eclipse computed different checksums. Our fix would be in PR https://github.com/ebean-orm/ebean/pull/2485

@jonasPoehler @rPraml

rPraml commented 2 years ago

@rbygrave do you already know, when/if you implement this feature. We ran into an issue, where we do some parameter replacement in the scripts and the checksum changes as soon the parameter changes.

In our case the parameter was used in tablespace definition (@Tablespace("${tablespace}")) and we need to change that. So many scripts were affected by checksum mismatch.

rbygrave commented 2 years ago

I have not yet made this change. I'm keen to do it but it keeps slipping ...

rPraml commented 1 year ago

Hello Rob, I think the change in #138 means there is an incompatible change to the use of checksums in combination with parameters. If you have time, please read the comments in #138 - Do you share my opinion or am I overlooking something?

Roland