Suggested solution for adding-not-nullable-field is a breaking change

[sazarubin]

Context

I've made a breaking change to a production database, which could be detectable with Squawk, then I've found my exact mistake as one of the solutions in the doc on the adding-not-nullable-field rule

Impact details

I was adding a new column that I want to make not-nullable, using

alter table
  abc add column xyz int null,
  add constraint xyz_not_null check (xyz is not null) not valid;

My concern was only about not blocking the writes to the table, but I completely missed the backwards compatibility, which is required when using a rolling deployment. Even worse is that rollback makes things worse in that case, and the only possible fix then is to drop the new constraint.

I was looking at the rules squawk provides to find the closest one and found the adding-not-nullable-field rule, which does acknowledge this change as backwards incompatible, but only in passing and then suggests exactly the code above as a fix.

My humble opinion

Since rolling deployments are a common practice, IMHO it makes sense to consider backwards incompatible changes a problem by default, so any time the column without a default is added along with virtually any constraint on it in the same release, we can suspect it to be a breaking change.

Another important point: adding the column and the constraint in the same statement or the same migration are as dangerous as doing it in different migrations but in the same release, which could be even more frequent. So it seems that the most value would be achieved, if the check would somehow run on the whole schema change, instead of individual file.

What is your opinion on this?

[sbdchd]

If I understand correctly, the scenario is:

-- initial state
create table abc(a int, b int);

-- migration running
alter table
  abc add column xyz int null,
  add constraint xyz_not_null check (xyz is not null) not valid;

-- meanwhile live traffic is writing without the new field
insert into abc values (3, 4);
-- starts erroring on the not valid constraint

So we should error when trying to add a column and also add a validation for that column in the same migration?

[sazarubin]

Exactly!

So we should error when trying to add a column and also add a validation for that column in the same migration?

Even better if it could be tracked across multiple migrations in the same release, but I'm not sure if that context is even available to squawk.