Re: NOT ENFORCED constraint feature

On Mon, Feb 10, 2025, at 7:03 AM, Amul Sul wrote:
> Attached patch implemented this behaviour. To achieve this, we have to
> revert (see 0007) some committed code and relax the restriction that
> the NOT ENFORCED constraint must also be NOT VALID. Now, NOT ENFORCED
> and NOT VALID are independent statuses, and the psql-meta meta command
> will display NOT VALID alongside the NOT ENFORCED constraint.
> Previously, we hid NOT VALID for NOT ENFORCED constraints, assuming it
> would be implied, but that is no longer the case.

I think this proposed state of affairs is problematic. Current queries that assume that pg_constraint.convalidated means that a constraint is validated would be broken. My suggestion at this point is that instead of adding a separate boolean column to pg_constraint we should be replacing `bool convalidated` with `char convalidity`, with new defines for all the possible states we require: enforced-and-valid ("V"alid), enforced-not-validated ("i"nvalid), not-enforced-and-not-valid (terribly "I"nvalid or maybe "U"nenforced), not-enforced-but-was-valid-before-turning-unenforced ("u"nenforced). Breaking user queries would make all apps reassess what do they actually want to know about the constraint without assumptions of how enforcement worked in existing Postgres releases.

This shouldn't be a very large change from what you currently have, I think.

> • v13-0001-Add-AlterConstraintStmt-struct-for-ALTER-.-CONST.patch

I think the new node name is wrong, because it makes the code looks as if we support ALTER CONSTRAINT as a statement for this, which is false. (This is a repeat of ReplicaIdentityStmt, which I think is a mistake). I would suggest a name like ATAlterConstraint instead. Perhaps we can use that in the patch for ALTER CONSTRAINT ... SET [NO] INHERIT as well, instead of (as I suggested Suraj) creating a separate subcommand number.