[Arne Roland <A.Roland@index.de>]

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

Attachments:

recursive_tgrename.patch (text/x-patch, 7.2K) download
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 (application/sql, 597B) download