Hello, On 2025-Feb-17, Amul Sul wrote:
> I have renamed AlterConstraintStmt to ATAlterConstraint as per your > suggestion in the attached version. Apart from this, there are no > other changes, as the final behavior is still unclear based on the > discussions so far. Okay, thanks. I've taken your alterDeferrability from your later patch (renamed to just "deferrability"). Also, IMO the routine structure needs a bit of a revamp. What do you think of the attached, which is mostly your 0001? (Actually, now that I look, it seems I made more or less the same changes you'd be doing in 0008, so this should be okay.) -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
>From 5b0cf18a0facd702b8006f38469344ea888c6de0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@alvh.no-ip.org> Date: Mon, 17 Feb 2025 18:20:30 +0100 Subject: [PATCH] Add ATAlterConstraint struct for ALTER .. CONSTRAINT. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the use of Constraint with the new ATAlterConstraint struct, which allows us to pass additional information, such as whether the constraint attribute is set. This is necessary for future patches that involve altering constraints in other ways. Author: Amul Sul <sula...@gmail.com> Author: Álvaro Herrera <alvhe...@alvh.no-ip.org> Discussion: https://postgr.es/m/CAAJ_b94bfgPV-8Mw_HwSBeheVwaK9=5s+7+kbbj_npwxqfg...@mail.gmail.com --- src/backend/commands/tablecmds.c | 160 ++++++++++++++++--------------- src/backend/parser/gram.y | 4 +- src/include/nodes/parsenodes.h | 25 +++-- src/tools/pgindent/typedefs.list | 1 + 4 files changed, 102 insertions(+), 88 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 72a1b64c2a2..072a6129963 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -389,17 +389,17 @@ static void AlterIndexNamespaces(Relation classRel, Relation rel, static void AlterSeqNamespaces(Relation classRel, Relation rel, Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved, LOCKMODE lockmode); -static ObjectAddress ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, +static ObjectAddress ATExecAlterConstraint(Relation rel, ATAlterConstraint *cmdcon, bool recurse, bool recursing, LOCKMODE lockmode); -static bool ATExecAlterConstrRecurse(Constraint *cmdcon, Relation conrel, Relation tgrel, - Relation rel, HeapTuple contuple, List **otherrelids, - LOCKMODE lockmode); +static bool ATExecAlterConstrDeferrability(ATAlterConstraint *cmdcon, Relation conrel, + Relation tgrel, Relation rel, HeapTuple contuple, + List **otherrelids, LOCKMODE lockmode); +static void ATExecAlterConstrDeferrabilityRecurse(ATAlterConstraint *cmdcon, Relation conrel, + Relation tgrel, Relation rel, HeapTuple contuple, + List **otherrelids, LOCKMODE lockmode); static void AlterConstrTriggerDeferrability(Oid conoid, Relation tgrel, Relation rel, bool deferrable, bool initdeferred, List **otherrelids); -static void ATExecAlterChildConstr(Constraint *cmdcon, Relation conrel, Relation tgrel, - Relation rel, HeapTuple contuple, List **otherrelids, - LOCKMODE lockmode); static ObjectAddress ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName, bool recurse, bool recursing, LOCKMODE lockmode); @@ -5450,7 +5450,9 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, lockmode); break; case AT_AlterConstraint: /* ALTER CONSTRAINT */ - address = ATExecAlterConstraint(rel, cmd, false, false, lockmode); + address = ATExecAlterConstraint(rel, castNode(ATAlterConstraint, + cmd->def), + false, false, lockmode); break; case AT_ValidateConstraint: /* VALIDATE CONSTRAINT */ address = ATExecValidateConstraint(wqueue, rel, cmd->name, cmd->recurse, @@ -11801,10 +11803,9 @@ GetForeignKeyCheckTriggers(Relation trigrel, * InvalidObjectAddress. */ static ObjectAddress -ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, bool recurse, +ATExecAlterConstraint(Relation rel, ATAlterConstraint *cmdcon, bool recurse, bool recursing, LOCKMODE lockmode) { - Constraint *cmdcon; Relation conrel; Relation tgrel; SysScanDesc scan; @@ -11813,9 +11814,6 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, bool recurse, Form_pg_constraint currcon; ObjectAddress address; List *otherrelids = NIL; - ListCell *lc; - - cmdcon = castNode(Constraint, cmd->def); conrel = table_open(ConstraintRelationId, RowExclusiveLock); tgrel = table_open(TriggerRelationId, RowExclusiveLock); @@ -11896,28 +11894,32 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, bool recurse, errhint("You may alter the constraint it derives from instead."))); } + address = InvalidObjectAddress; + /* * Do the actual catalog work. We can skip changing if already in the * desired state, but not if a partitioned table: partitions need to be * processed regardless, in case they had the constraint locally changed. */ - address = InvalidObjectAddress; - if (currcon->condeferrable != cmdcon->deferrable || - currcon->condeferred != cmdcon->initdeferred || - rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + if (cmdcon->deferrability) { - if (ATExecAlterConstrRecurse(cmdcon, conrel, tgrel, rel, contuple, - &otherrelids, lockmode)) - ObjectAddressSet(address, ConstraintRelationId, currcon->oid); - } + if (currcon->condeferrable != cmdcon->deferrable || + currcon->condeferred != cmdcon->initdeferred || + rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + if (ATExecAlterConstrDeferrability(cmdcon, conrel, tgrel, rel, contuple, + &otherrelids, lockmode)) + ObjectAddressSet(address, ConstraintRelationId, currcon->oid); + } - /* - * ATExecAlterConstrRecurse already invalidated relcache for the relations - * having the constraint itself; here we also invalidate for relations - * that have any triggers that are part of the constraint. - */ - foreach(lc, otherrelids) - CacheInvalidateRelcacheByRelid(lfirst_oid(lc)); + /* + * ATExecAlterConstrDeferrability already invalidated relcache for the + * relations having the constraint itself; here we also invalidate for + * relations that have any triggers that are part of the constraint. + */ + foreach_oid(relid, otherrelids) + CacheInvalidateRelcacheByRelid(relid); + } systable_endscan(scan); @@ -11939,9 +11941,9 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, bool recurse, * but existing releases don't do that.) */ static bool -ATExecAlterConstrRecurse(Constraint *cmdcon, Relation conrel, Relation tgrel, - Relation rel, HeapTuple contuple, List **otherrelids, - LOCKMODE lockmode) +ATExecAlterConstrDeferrability(ATAlterConstraint *cmdcon, Relation conrel, + Relation tgrel, Relation rel, HeapTuple contuple, + List **otherrelids, LOCKMODE lockmode) { Form_pg_constraint currcon; Oid conoid; @@ -12000,18 +12002,61 @@ ATExecAlterConstrRecurse(Constraint *cmdcon, Relation conrel, Relation tgrel, */ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE || get_rel_relkind(refrelid) == RELKIND_PARTITIONED_TABLE) - ATExecAlterChildConstr(cmdcon, conrel, tgrel, rel, contuple, - otherrelids, lockmode); + ATExecAlterConstrDeferrabilityRecurse(cmdcon, conrel, tgrel, rel, contuple, + otherrelids, lockmode); return changed; } /* - * A subroutine of ATExecAlterConstrRecurse that updated constraint trigger's - * deferrability. + * Invokes ATExecAlterConstrDeferrability for each constraint that is a child + * of the specified constraint. * * The arguments to this function have the same meaning as the arguments to - * ATExecAlterConstrRecurse. + * ATExecAlterConstrDeferrability. + */ +static void +ATExecAlterConstrDeferrabilityRecurse(ATAlterConstraint *cmdcon, Relation conrel, + Relation tgrel, Relation rel, HeapTuple contuple, + List **otherrelids, LOCKMODE lockmode) +{ + Form_pg_constraint currcon; + Oid conoid; + ScanKeyData pkey; + SysScanDesc pscan; + HeapTuple childtup; + + currcon = (Form_pg_constraint) GETSTRUCT(contuple); + conoid = currcon->oid; + + ScanKeyInit(&pkey, + Anum_pg_constraint_conparentid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(conoid)); + + pscan = systable_beginscan(conrel, ConstraintParentIndexId, + true, NULL, 1, &pkey); + + while (HeapTupleIsValid(childtup = systable_getnext(pscan))) + { + Form_pg_constraint childcon = (Form_pg_constraint) GETSTRUCT(childtup); + Relation childrel; + + childrel = table_open(childcon->conrelid, lockmode); + ATExecAlterConstrDeferrability(cmdcon, conrel, tgrel, childrel, childtup, + otherrelids, lockmode); + table_close(childrel, NoLock); + } + + systable_endscan(pscan); +} + +/* + * A subroutine of ATExecAlterConstrDeferrability that updated constraint + * trigger's deferrability. + * + * The arguments to this function have the same meaning as the arguments to + * ATExecAlterConstrDeferrability. */ static void AlterConstrTriggerDeferrability(Oid conoid, Relation tgrel, Relation rel, @@ -12071,49 +12116,6 @@ AlterConstrTriggerDeferrability(Oid conoid, Relation tgrel, Relation rel, systable_endscan(tgscan); } -/* - * Invokes ATExecAlterConstrRecurse for each constraint that is a child of the - * specified constraint. - * - * The arguments to this function have the same meaning as the arguments to - * ATExecAlterConstrRecurse. - */ -static void -ATExecAlterChildConstr(Constraint *cmdcon, Relation conrel, Relation tgrel, - Relation rel, HeapTuple contuple, List **otherrelids, - LOCKMODE lockmode) -{ - Form_pg_constraint currcon; - Oid conoid; - ScanKeyData pkey; - SysScanDesc pscan; - HeapTuple childtup; - - currcon = (Form_pg_constraint) GETSTRUCT(contuple); - conoid = currcon->oid; - - ScanKeyInit(&pkey, - Anum_pg_constraint_conparentid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(conoid)); - - pscan = systable_beginscan(conrel, ConstraintParentIndexId, - true, NULL, 1, &pkey); - - while (HeapTupleIsValid(childtup = systable_getnext(pscan))) - { - Form_pg_constraint childcon = (Form_pg_constraint) GETSTRUCT(childtup); - Relation childrel; - - childrel = table_open(childcon->conrelid, lockmode); - ATExecAlterConstrRecurse(cmdcon, conrel, tgrel, childrel, childtup, - otherrelids, lockmode); - table_close(childrel, NoLock); - } - - systable_endscan(pscan); -} - /* * ALTER TABLE VALIDATE CONSTRAINT * diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index d3887628d46..268db669c83 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -2657,12 +2657,12 @@ alter_table_cmd: | ALTER CONSTRAINT name ConstraintAttributeSpec { AlterTableCmd *n = makeNode(AlterTableCmd); - Constraint *c = makeNode(Constraint); + ATAlterConstraint *c = makeNode(ATAlterConstraint); n->subtype = AT_AlterConstraint; n->def = (Node *) c; - c->contype = CONSTR_FOREIGN; /* others not supported, yet */ c->conname = $3; + c->deferrability = true; processCASbits($4, @4, "ALTER CONSTRAINT statement", &c->deferrable, &c->initdeferred, diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 8dd421fa0ef..eec7b22d0b1 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2469,13 +2469,6 @@ typedef enum AlterTableType AT_ReAddStatistics, /* internal to commands/tablecmds.c */ } AlterTableType; -typedef struct ReplicaIdentityStmt -{ - NodeTag type; - char identity_type; - char *name; -} ReplicaIdentityStmt; - typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */ { NodeTag type; @@ -2492,6 +2485,24 @@ typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */ bool recurse; /* exec-time recursion */ } AlterTableCmd; +/* Ad-hoc node for AT_AlterConstraint */ +typedef struct ATAlterConstraint +{ + NodeTag type; + char *conname; /* Constraint name */ + bool deferrability; /* changing deferrability properties? */ + bool deferrable; /* DEFERRABLE? */ + bool initdeferred; /* INITIALLY DEFERRED? */ +} ATAlterConstraint; + +/* Ad-hoc node for AT_ReplicaIdentity */ +typedef struct ReplicaIdentityStmt +{ + NodeTag type; + char identity_type; + char *name; +} ReplicaIdentityStmt; + /* ---------------------- * Alter Collation diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index bce4214503d..00d6a5aebac 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -157,6 +157,7 @@ ArrayType AsyncQueueControl AsyncQueueEntry AsyncRequest +ATAlterConstraint AttInMetadata AttStatsSlot AttoptCacheEntry -- 2.39.5
