sql: bug when setting a column NOT NULL and then dropping it

[thoszhang]

This was uncovered from looking at logs for TestRandomSyntaxSchemaChangeColumn. If a column is dropped while a schema change for SET NOT NULL is in progress on the same column, the validation for the non-null column (which is a SQL query) can fail with the error column %q does not exist. The column drop can be queued after SET NOT NULL with a different mutation ID, or it can even happen if the two schema changes happen in the same transaction:

[email protected]:62055/movr> create table t (a int);
CREATE TABLE

Time: 3.051ms

[email protected]:62055/movr> begin; alter table t alter column a set not null; alter table t drop column a; commit;
ERROR: transaction committed but schema change aborted with error: (42703): validation of NOT NULL constraint failed: validate check constraint: column "a" does not exist
SQLSTATE: XXA00
HINT: Some of the non-DDL statements may have committed successfully, but some of the DDL statement(s) failed.
Manual inspection may be required to determine the actual state of the database.
--
See: https://github.com/cockroachdb/cockroach/issues/42061

This particular bug is not very severe; failing with an error is what we should be doing in this case, even though it's not a very good error. The larger question is what we can do to avoid bugs related to interactions between in-progress schema changes and schema changes that are later in line. We're currently dealing with these interactions on an ad-hoc basis, which is going to inevitably lead to similar bugs.

[thoszhang]

This would be fixed with the approach outlined in #47989 (comment).

[ajwerner]

It turns out this is much more severe if the DROP COLUMN is concurrent but in a different transaction and thus has a different mutation ID.

[ajwerner]

@fqazi see #61500 (comment).

I think we can do something about this without too too much effort. Namely I think we want to skip over some set of mutations which are made irrelevant by some set of later mutations. Specifically I'm thinking about constraints like not null and check that have to do with a now dropped column.

It's possible that we should just straight up drop those mutations when we're dropping the column. Then, we can augment that by being a bit more careful about the code that relies on those mutations existing. All of this is unsatisfying but I think these bugs are worth doing something about even if it's only for the short term. We'll want to backport whatever we do here I suspect.