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
meansthat a constraint is validated would be broken. My suggestion at this point is that instead of adding a separate
booleancolumn to pg_constraint we should be replacing `bool convalidated` with `char convalidity`, with new defines for
allthe 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
whatdo they actually want to know about the constraint without assumptions of how enforcement worked in existing
Postgresreleases.
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
forthis, which is false. (This is a repeat of ReplicaIdentityStmt, which I think is a mistake). I would suggest a
namelike 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.
