Hello,
I got too annoyed at building queries for gexec all the time. So wrote a patch to fix the issue that the rename of partitioned trigger doesn't affect children. In case there isn't a dependent trigger with the same name on the child the function errors out. If the user touched the trigger of the child, it's not a sensitive idea to mess with it. I'm not happy with the interface change in trigger.h. Yet I don't like the idea of creating another layer of indirection either. Even though it's a small patch, there might be other things I'm missing, so I'd appreciate feedback. Regards Arne
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index b11ebf0f61..e92dae78f7 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -365,7 +365,7 @@ ExecRenameStmt(RenameStmt *stmt) stmt->newname); case OBJECT_TRIGGER: - return renametrig(stmt); + return renametrig(stmt, 0, 0); case OBJECT_POLICY: return rename_policy(stmt); diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index c336b238aa..039386958a 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -1357,12 +1357,7 @@ get_trigger_oid(Oid relid, const char *trigname, bool missing_ok) return oid; } -/* - * Perform permissions and integrity checks before acquiring a relation lock. - */ -static void -RangeVarCallbackForRenameTrigger(const RangeVar *rv, Oid relid, Oid oldrelid, - void *arg) +static void callbackForRenameTrigger(char* relname, Oid relid) { HeapTuple tuple; Form_pg_class form; @@ -1379,22 +1374,32 @@ RangeVarCallbackForRenameTrigger(const RangeVar *rv, Oid relid, Oid oldrelid, ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("\"%s\" is not a table, view, or foreign table", - rv->relname))); + relname))); /* you must own the table to rename one of its triggers */ if (!pg_class_ownercheck(relid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relid)), rv->relname); + aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relid)), relname); if (!allowSystemTableMods && IsSystemClass(relid, form)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied: \"%s\" is a system catalog", - rv->relname))); + relname))); ReleaseSysCache(tuple); } /* - * renametrig - changes the name of a trigger on a relation + * Perform permissions and integrity checks before acquiring a relation lock. + */ +static void +RangeVarCallbackForRenameTrigger(const RangeVar *rv, Oid relid, Oid oldrelid, + void *arg) +{ + callbackForRenameTrigger(rv->relname, relid); +} + +/* + * renameonlytrig - changes the name of a trigger on a relation * * trigger name is changed in trigger catalog. * No record of the previous name is kept. @@ -1407,41 +1412,26 @@ RangeVarCallbackForRenameTrigger(const RangeVar *rv, Oid relid, Oid oldrelid, * update row in catalog */ ObjectAddress -renametrig(RenameStmt *stmt) +renameonlytrig(RenameStmt *stmt, Relation tgrel, Oid relid, Oid tgparent) { Oid tgoid; Relation targetrel; - Relation tgrel; HeapTuple tuple; SysScanDesc tgscan; ScanKeyData key[2]; - Oid relid; ObjectAddress address; - /* - * Look up name, check permissions, and acquire lock (which we will NOT - * release until end of transaction). - */ - relid = RangeVarGetRelidExtended(stmt->relation, AccessExclusiveLock, - 0, - RangeVarCallbackForRenameTrigger, - NULL); /* Have lock already, so just need to build relcache entry. */ targetrel = relation_open(relid, NoLock); + /* * Scan pg_trigger twice for existing triggers on relation. We do this in * order to ensure a trigger does not exist with newname (The unique index * on tgrelid/tgname would complain anyway) and to ensure a trigger does * exist with oldname. * - * NOTE that this is cool only because we have AccessExclusiveLock on the - * relation, so the trigger set won't be changing underneath us. - */ - tgrel = table_open(TriggerRelationId, RowExclusiveLock); - - /* * First pass -- look for name conflict */ ScanKeyInit(&key[0], @@ -1472,6 +1462,7 @@ renametrig(RenameStmt *stmt) Anum_pg_trigger_tgname, BTEqualStrategyNumber, F_NAMEEQ, PointerGetDatum(stmt->subname)); + tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true, NULL, 2, key); if (HeapTupleIsValid(tuple = systable_getnext(tgscan))) @@ -1484,7 +1475,12 @@ renametrig(RenameStmt *stmt) tuple = heap_copytuple(tuple); /* need a modifiable copy */ trigform = (Form_pg_trigger) GETSTRUCT(tuple); tgoid = trigform->oid; - + if (tgparent != 0 && tgparent != trigform->tgparentid) { + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("trigger \"%s\" for table \"%s\" is not dependent on the trigger on \"%s\"", + stmt->subname, RelationGetRelationName(targetrel), stmt->relation->relname))); + } namestrcpy(&trigform->tgname, stmt->newname); @@ -1522,6 +1518,73 @@ renametrig(RenameStmt *stmt) return address; } +/* + * renametrig - changes the name of a trigger on a possibly partitioned relation recurisvely + * + * alter name of parent trigger using renameonlytrig + * recursive over children and change their names in the same manner + * + */ +ObjectAddress +renametrig(RenameStmt *stmt, Oid tgoid, Oid relid) +{ + + Relation tgrel; + Oid newtgoid; + ObjectAddress tgaddress; + /* + * NOTE that this is cool only because we have AccessExclusiveLock on the + * relation, so the trigger set won't be changing underneath us. + */ + tgrel = table_open(TriggerRelationId, RowExclusiveLock); + + /* + * Look up name, check permissions, and acquire lock (which we will NOT + * release until end of transaction). + * If relid is set the last call of renamtrig already locked this relation. + */ + if (relid == 0) { + relid = RangeVarGetRelidExtended(stmt->relation, AccessExclusiveLock, + 0, + RangeVarCallbackForRenameTrigger, + NULL); + } else { + callbackForRenameTrigger(NULL, relid); + LockRelationOid(relid, AccessExclusiveLock); + } + tgaddress = renameonlytrig(stmt, tgrel, relid, tgoid); + newtgoid = tgaddress.objectId; + + /* + * In case we have a partioned table we traverse them and rename every dependend subtrigger + * of the same name or error out if it doesn't exist. + */ + if (has_subclass(relid)) + { + ListCell *child; + List *children; + + /* Make sure it stays a child. */ + children = find_inheritance_children(relid, AccessExclusiveLock); + + /* + * find_all_inheritors does the recursive search of the inheritance + * hierarchy, so all we have to do is process all of the relids in the + * list that it returns. + */ + foreach(child, children) + { + Oid childrelid = lfirst_oid(child); + + if (childrelid == relid) + continue; + renametrig(stmt, newtgoid, childrelid); + } + } + return tgaddress; +} + + /* * EnableDisableTrigger() diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index 244540d90b..8e8eb98ec0 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -158,7 +158,9 @@ extern ObjectAddress CreateTrigger(CreateTrigStmt *stmt, const char *queryString extern void RemoveTriggerById(Oid trigOid); extern Oid get_trigger_oid(Oid relid, const char *name, bool missing_ok); -extern ObjectAddress renametrig(RenameStmt *stmt); +extern ObjectAddress renametrig(RenameStmt *stmt, Oid tgoid, Oid relid); + +extern ObjectAddress renameonlytrig(RenameStmt *stmt, Relation tgrel, Oid relid, Oid tgparent); extern void EnableDisableTrigger(Relation rel, const char *tgname, char fires_when, bool skip_system, LOCKMODE lockmode);
partitioned_tgrename.sql
Description: partitioned_tgrename.sql