codd icon indicating copy to clipboard operation
codd copied to clipboard

Consider running `RESET ALL` after each migration

Open mzabani opened this issue 2 years ago • 1 comments

The problem

Because of session settings, migrations do not compose well. Suppose there are two migrations:

Mig A:

SET ROLE otherrole;

Mig B:

CREATE TABLE sometbl();

The ownership of sometbl will differ if migration B is applied in the same transaction as Mig A, compared to them being applied in two distinct batches.

This is a huge risk, since deployments to other environments can in some cases have both migrations and in others not, leading to different schemas.

The same thing is true of any session or transaction level setting; it's not just roles.

A possible solution

One possible solution is for codd to run RESET ALL; RESET ROLE after each migration. Does this reset SET SESSION AUTHORIZATION too? That should also be reset.

Does it work, though?

  • Does it matter if we're in a transaction or not?
  • Are there other things that can affect migration composability other than session settings?
  • Is it possible, for instance, to update pg_settings and change reset values? Or maybe just ALTER DATABASE ... SET .. is sufficient for that?
  • What is the expectation of users? What mental model do they apply instinctively when looking at codd?
    • This might not matter much unless it's a landslide victory for the mental model where users expect SET .. to have effects on other migrations. Making sure different developers working in the same codebase write migrations without fear of affecting others or even accidentally forgetting SET .. can affect others seems more important.

I need to read https://www.postgresql.org/docs/13/config-setting.html#CONFIG-SETTING-SQL-COMMAND-INTERACTION and run experiments before deciding.

mzabani avatar Oct 23 '22 14:10 mzabani

~I don't think this actually works because we also checksum pg_settings. If we RESET ALL, we undo any SET .. and thus will not be able to checksum changes in settings. None of this would be a problem if we could checksum changes made with ALTER DATABASE .. SET .. TO .. without requiring SET, so it might be worth investigating that a bit more.~

https://github.com/mzabani/codd/pull/161 means users no longer need to add SET .. to their migrations to have their database settings checked by codd.

mzabani avatar Nov 03 '22 20:11 mzabani