kysely-org / kysely

A type-safe typescript SQL query builder
https://kysely.dev
MIT License
10.46k stars 268 forks source link

Migrations can be modified without notice #984

Open niklas-e opened 4 months ago

niklas-e commented 4 months ago

Description:

I noticed migrator stores only the migration name into db. Therefore if there's any changes in migrations, you wouldn't notice it in environments where you have ran them already.

Expected behaviour:

Migrator throws an error if the current migration contents do not match already ran migrations.

Possible solution:

This could be potentially solved by adding a checksum of the migration to the migration table and verifying it while running migrations.

alenap93 commented 4 months ago

I think that a solution could be a checksum system like liquibase

Bessonov commented 4 months ago

I have used Liquibase in the past, but now in a project I am migrating away from it to Kysely. While I understand the benefits of integrity checks, especially at scale, I am against them or, at least, in favor of having the option to ignore or reset the checks. I have the following reasons:

Therefore, please do not take my reaction to the feature request personally, but as a vote.

koskimas commented 4 months ago

I completely agree with @Bessonov! We could add the check behind an option (off by default) but that would be tricky since we'd have to migrate the migration tables. I don't think there's a way to do that in a way that'd work on all possible dialects (internal and 3rd party). We'd have to add that to the dialects, making them more complex.

boehs commented 3 months ago

I'm curious what the use case would even be for this? Kysely's value to me as a programmer is to help me avoid unexpected things, but when I change my migration that is absolutely an expected change

niklas-e commented 3 months ago

IMO these are the most important reasons for having integrity checks on your database migrations.


  1. Ensuring consistency

Checksums ensure that the migration scripts remain unchanged from development to deployment. Any modifications to the scripts after they have been executed in a given environment will be detected, ensuring that all environments (development, testing, staging, production) remain consistent. I.e. Helps in preventing schema drift if scripts are altered or not applied uniformly.

  1. Audit trails

Having a reliable checksum system helps maintain an audit trail of all changes applied to the database. This can be crucial for compliance and auditing purposes, as it provides a verifiable history of migrations.

  1. Collaboration

In a team environment, multiple developers might work on the database schema simultaneously. Checksums help ensure that everyone is working with the same version of the migration scripts, reducing conflicts and integration issues.

  1. Reliable rollbacks

This is not so important for me personally as I usually prefer forward-only migration model. However, if a migration fails or causes issues, checksums can help ensure that rollbacks are executed correctly by verifying that the scripts being rolled back are the same as the ones initially run.


To summarize, I think it's all about maintaining high standards of database integrity, ensuring reliable deployments and minimizing risks associated with schema changes.

GioPat commented 3 months ago

I strongly agree with @niklas-e, I'm using liquibase for a quite big project and sometimes the integrity checks on the migrations saved my day. If you introduce utilities like changelog sync you can solve the issues mentioned by @Bessonov.

Bessonov commented 3 months ago

@niklas-e In fact, none of the mentioned issues are resolved by checking the execution integrity. It doesn't prevent any schema drifts. Go into your database, drop a random column, and it wouldn't be detected by the "integrity checks" done by Liquibase. The only thing it can detect is if the migration files are different from the past migrations.

If you want an integrity check, then the tool must check the migration files against the actual database schema (again, not just the checksums table!). I see that Liquibase Pro could be a solution. However, this is an entirely different league and not what you are asking for in this issue.

If you need auditing, then, by definition, a client-side database migration tool isn't the right tool for that. Either it must be an inherent property of the database, or you must ensure that it can't be faked by developers and use, for example, Audit Triggers with appropriate permissions. Of course, there are possible workarounds. However, you can also apply workarounds to ensure that old files are unmodified, and you don't even need Kysely for that. See below.

I strongly agree with @niklas-e. I'm using Liquibase for a quite big project and sometimes the integrity checks on the migrations saved my day. If you introduce utilities like changelog sync, you can solve the issues mentioned by @Bessonov.

@GioPat No, they don't. Again, until you compare the actual schema with the migration files, your gain is nearly nonexistent. Not tested, but probably, I think the following should do the same for you as checksum checks:

if [ $(git diff main..HEAD --name-only --diff-filter=MD | grep '.*\.sql$' | wc -l) -gt 0 ]; then
  echo Integrity check failed
  exit 1
fi

Adjust it to your needs. Thanks me later :smile:

niklas-e commented 3 months ago

You are right, they are not a standalone solution which solves everything, and I didn't claim them to be such. However I still think they are very useful.

You are implying database schema can be changed by other means than migration files. That is not the case in many environments.