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
