Skip site navigation (1) Skip section navigation (2)

Re: RENAME TRIGGER patch (was [HACKERS] Odd(?) RI-trigger

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>,Alvaro Herrera <alvherre(at)atentus(dot)com>,Stephan Szabo <sszabo(at)megazone23(dot)bigpanda(dot)com>,pgsql-patches(at)postgresql(dot)org
Subject: Re: RENAME TRIGGER patch (was [HACKERS] Odd(?) RI-trigger
Date: 2002-04-23 18:07:19
Message-ID: 200204231807.g3NI7Jd22262@candle.pha.pa.us (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
Your patch has been added to the PostgreSQL unapplied patches list at:

	http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------


Joe Conway wrote:
> Tom Lane wrote:
>  > Joe Conway <mail(at)joeconway(dot)com> writes:
>  >
>  >> There is already a RenameStmt node which is currently only used to
>  >>  rename tables or table column names. Is there any objection to
>  >> modifying it to handle trigger names (and possibly other things in
>  >> the future) also?
>  >
>  >
>  > You'd need to add a field so you could distinguish the type of
>  > rename, but on the whole that seems a reasonable thing to do;
>  > probably better than adding a brand new node type.  We're already
>  > sharing node types for DROPs, for example, so I see no reason not to
>  > do it for RENAMEs. (Cf 'DropPropertyStmt' in current sources)
>  >
>  > Renaming rules seems like something that should be on the list too, so
>  > you're right that there will be more stuff later.
>  >
> 
> Attached is a patch for ALTER TRIGGER RENAME per the above thread. I
> left a stub for a future "ALTER RULE RENAME" but did not write that one
> yet. Bruce, if you want to add my name for for that I'll take it and do
> it later.
> 
> It passes all regression tests on my RH box. Usage is as follows:
> 
> test=# create table foo3(f1 int references foo2(f1));
> NOTICE:  CREATE TABLE will create implicit trigger(s) for FOREIGN KEY
> check(s)
> CREATE
> test=# \d foo3
>           Table "foo3"
>   Column |  Type   | Modifiers
> --------+---------+-----------
>   f1     | integer |
> Triggers: RI_ConstraintTrigger_16663
> 
> test=# alter trigger "RI_ConstraintTrigger_16663" on foo3 rename to
> "MyOwnConstTriggerName";
> ALTER
> test=# \d foo3
>           Table "foo3"
>   Column |  Type   | Modifiers
> --------+---------+-----------
>   f1     | integer |
> Triggers: MyOwnConstTriggerName
> 
> Obviously there is no build in restriction on altering the name of
> refint triggers -- is this a problem?
> 
> I'll follow up with a doc patch this weekend. If there are no 
> objections, please apply.
> 
> Thanks,
> 
> Joe

> diff -cNr pgsql.cvs.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
> *** pgsql.cvs.orig/src/backend/commands/tablecmds.c	Fri Apr 19 10:32:50 2002
> --- pgsql/src/backend/commands/tablecmds.c	Fri Apr 19 16:46:11 2002
> ***************
> *** 2851,2856 ****
> --- 2851,2973 ----
>   }
>   
>   /*
> +  *		renametrig		- changes the name of a trigger on a relation
> +  *
> +  *		trigger name is changed in trigger catalog.
> +  *		No record of the previous name is kept.
> +  *
> +  *		get proper relrelation from relation catalog (if not arg)
> +  *		scan trigger catalog
> +  *				for name conflict (within rel)
> +  *				for original trigger (if not arg)
> +  *		modify tgname in trigger tuple
> +  *		insert modified trigger in trigger catalog
> +  *		delete original trigger from trigger catalog
> +  */
> + extern void renametrig(Oid relid,
> + 		  const char *oldname,
> + 		  const char *newname)
> + {
> + 	Relation	targetrel;
> + 	Relation	tgrel;
> + 	HeapTuple	tuple;
> + 	SysScanDesc	tgscan;
> + 	ScanKeyData key;
> + 	bool		found = FALSE;
> + 	Relation	idescs[Num_pg_trigger_indices];
> + 
> + 	/*
> + 	 * Grab an exclusive lock on the target table, which we will NOT
> + 	 * release until end of transaction.
> + 	 */
> + 	targetrel = heap_open(relid, AccessExclusiveLock);
> + 
> + 	/*
> + 	 * 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 = heap_openr(TriggerRelationName, RowExclusiveLock);
> + 
> + 	/*
> + 	 * First pass -- look for name conflict
> + 	 */
> + 	ScanKeyEntryInitialize(&key, 0,
> + 						   Anum_pg_trigger_tgrelid,
> + 						   F_OIDEQ,
> + 						   ObjectIdGetDatum(relid));
> + 	tgscan = systable_beginscan(tgrel, TriggerRelidNameIndex, true,
> + 								SnapshotNow, 1, &key);
> + 	while (HeapTupleIsValid(tuple = systable_getnext(tgscan)))
> + 	{
> + 		Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(tuple);
> + 
> + 		if (namestrcmp(&(pg_trigger->tgname), newname) == 0)
> + 			elog(ERROR, "renametrig: trigger %s already defined on relation %s",
> + 				 newname, RelationGetRelationName(targetrel));
> + 	}
> + 	systable_endscan(tgscan);
> + 
> + 	/*
> + 	 * Second pass -- look for trigger existing with oldname and update
> + 	 */
> + 	ScanKeyEntryInitialize(&key, 0,
> + 						   Anum_pg_trigger_tgrelid,
> + 						   F_OIDEQ,
> + 						   ObjectIdGetDatum(relid));
> + 	tgscan = systable_beginscan(tgrel, TriggerRelidNameIndex, true,
> + 								SnapshotNow, 1, &key);
> + 	while (HeapTupleIsValid(tuple = systable_getnext(tgscan)))
> + 	{
> + 		Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(tuple);
> + 
> + 		if (namestrcmp(&(pg_trigger->tgname), oldname) == 0)
> + 		{
> + 			/*
> + 			 * Update pg_trigger tuple with new tgname.
> + 			 * (Scribbling on tuple is OK because it's a copy...)
> + 			 */
> + 			namestrcpy(&(pg_trigger->tgname), newname);
> + 			simple_heap_update(tgrel, &tuple->t_self, tuple);
> + 
> + 			/*
> + 			 * keep system catalog indices current
> + 			 */
> + 			CatalogOpenIndices(Num_pg_trigger_indices, Name_pg_trigger_indices, idescs);
> + 			CatalogIndexInsert(idescs, Num_pg_trigger_indices, tgrel, tuple);
> + 			CatalogCloseIndices(Num_pg_trigger_indices, idescs);
> + 
> + 			/*
> + 			 * Invalidate relation's relcache entry so that other
> + 			 * backends (and this one too!) are sent SI message to make them
> + 			 * rebuild relcache entries.
> + 			 */
> + 			CacheInvalidateRelcache(relid);
> + 
> + 			found = TRUE;
> + 			break;
> + 		}
> + 	}
> + 	systable_endscan(tgscan);
> + 
> + 	heap_close(tgrel, RowExclusiveLock);
> + 
> + 	if (!found)
> + 		elog(ERROR, "renametrig: trigger %s not defined on relation %s",
> + 			 oldname, RelationGetRelationName(targetrel));
> + 
> + 	/*
> + 	 * Close rel, but keep exclusive lock!
> + 	 */
> + 	heap_close(targetrel, NoLock);
> + }
> + 
> + 
> + /*
>    * Given a trigger function OID, determine whether it is an RI trigger,
>    * and if so whether it is attached to PK or FK relation.
>    *
> diff -cNr pgsql.cvs.orig/src/backend/nodes/copyfuncs.c pgsql/src/backend/nodes/copyfuncs.c
> *** pgsql.cvs.orig/src/backend/nodes/copyfuncs.c	Fri Apr 19 10:32:51 2002
> --- pgsql/src/backend/nodes/copyfuncs.c	Fri Apr 19 13:58:47 2002
> ***************
> *** 2137,2146 ****
>   	RenameStmt *newnode = makeNode(RenameStmt);
>   
>   	Node_Copy(from, newnode, relation);
> ! 	if (from->column)
> ! 		newnode->column = pstrdup(from->column);
>   	if (from->newname)
>   		newnode->newname = pstrdup(from->newname);
>   
>   	return newnode;
>   }
> --- 2137,2147 ----
>   	RenameStmt *newnode = makeNode(RenameStmt);
>   
>   	Node_Copy(from, newnode, relation);
> ! 	if (from->oldname)
> ! 		newnode->oldname = pstrdup(from->oldname);
>   	if (from->newname)
>   		newnode->newname = pstrdup(from->newname);
> + 	newnode->renameType = from->renameType;
>   
>   	return newnode;
>   }
> diff -cNr pgsql.cvs.orig/src/backend/nodes/equalfuncs.c pgsql/src/backend/nodes/equalfuncs.c
> *** pgsql.cvs.orig/src/backend/nodes/equalfuncs.c	Fri Apr 19 10:32:51 2002
> --- pgsql/src/backend/nodes/equalfuncs.c	Fri Apr 19 13:56:00 2002
> ***************
> *** 983,992 ****
>   {
>   	if (!equal(a->relation, b->relation))
>   		return false;
> ! 	if (!equalstr(a->column, b->column))
>   		return false;
>   	if (!equalstr(a->newname, b->newname))
>   		return false;
>   
>   	return true;
>   }
> --- 983,994 ----
>   {
>   	if (!equal(a->relation, b->relation))
>   		return false;
> ! 	if (!equalstr(a->oldname, b->oldname))
>   		return false;
>   	if (!equalstr(a->newname, b->newname))
>   		return false;
> + 	if (a->renameType != b->renameType)
> + 		return false;
>   
>   	return true;
>   }
> diff -cNr pgsql.cvs.orig/src/backend/parser/gram.y pgsql/src/backend/parser/gram.y
> *** pgsql.cvs.orig/src/backend/parser/gram.y	Fri Apr 19 10:32:51 2002
> --- pgsql/src/backend/parser/gram.y	Fri Apr 19 14:07:35 2002
> ***************
> *** 2915,2922 ****
>   				{
>   					RenameStmt *n = makeNode(RenameStmt);
>   					n->relation = $3;
> ! 					n->column = $6;
>   					n->newname = $8;
>   					$$ = (Node *)n;
>   				}
>   		;
> --- 2915,2935 ----
>   				{
>   					RenameStmt *n = makeNode(RenameStmt);
>   					n->relation = $3;
> ! 					n->oldname = $6;
>   					n->newname = $8;
> + 					if ($6 == NULL)
> + 						n->renameType = RENAME_TABLE;
> + 					else
> + 						n->renameType = RENAME_COLUMN;
> + 					$$ = (Node *)n;
> + 				}
> + 		| ALTER TRIGGER name ON relation_expr RENAME TO name
> + 				{
> + 					RenameStmt *n = makeNode(RenameStmt);
> + 					n->relation = $5;
> + 					n->oldname = $3;
> + 					n->newname = $8;
> + 					n->renameType = RENAME_TRIGGER;
>   					$$ = (Node *)n;
>   				}
>   		;
> diff -cNr pgsql.cvs.orig/src/backend/tcop/utility.c pgsql/src/backend/tcop/utility.c
> *** pgsql.cvs.orig/src/backend/tcop/utility.c	Fri Apr 19 10:32:52 2002
> --- pgsql/src/backend/tcop/utility.c	Fri Apr 19 15:59:13 2002
> ***************
> *** 377,399 ****
>   
>   				CheckOwnership(stmt->relation, true);
>   
> ! 				if (stmt->column == NULL)
>   				{
> ! 					/*
> ! 					 * rename relation
> ! 					 */
> ! 					renamerel(RangeVarGetRelid(stmt->relation, false),
> ! 							  stmt->newname);
> ! 				}
> ! 				else
> ! 				{
> ! 					/*
> ! 					 * rename attribute
> ! 					 */
> ! 					renameatt(RangeVarGetRelid(stmt->relation, false),
> ! 							  stmt->column,		/* old att name */
>   							  stmt->newname,	/* new att name */
> ! 							  interpretInhOption(stmt->relation->inhOpt));		/* recursive? */
>   				}
>   			}
>   			break;
> --- 377,406 ----
>   
>   				CheckOwnership(stmt->relation, true);
>   
> ! 				switch (stmt->renameType)
>   				{
> ! 					case RENAME_TABLE:
> ! 						renamerel(RangeVarGetRelid(stmt->relation, false),
> ! 								  stmt->newname);
> ! 						break;
> ! 					case RENAME_COLUMN:
> ! 						renameatt(RangeVarGetRelid(stmt->relation, false),
> ! 							  stmt->oldname,	/* old att name */
>   							  stmt->newname,	/* new att name */
> ! 							  interpretInhOption(stmt->relation->inhOpt));	/* recursive? */
> ! 						break;
> ! 					case RENAME_TRIGGER:
> ! 						renametrig(RangeVarGetRelid(stmt->relation, false),
> ! 							  stmt->oldname,	/* old att name */
> ! 							  stmt->newname);	/* new att name */
> ! 						break;
> ! 					case RENAME_RULE:
> ! 						elog(ERROR, "ProcessUtility: Invalid target for RENAME: %d",
> ! 								stmt->renameType);
> ! 						break;
> ! 					default:
> ! 						elog(ERROR, "ProcessUtility: Invalid target for RENAME: %d",
> ! 								stmt->renameType);
>   				}
>   			}
>   			break;
> diff -cNr pgsql.cvs.orig/src/include/commands/tablecmds.h pgsql/src/include/commands/tablecmds.h
> *** pgsql.cvs.orig/src/include/commands/tablecmds.h	Fri Apr 19 10:32:55 2002
> --- pgsql/src/include/commands/tablecmds.h	Fri Apr 19 16:06:39 2002
> ***************
> *** 15,20 ****
> --- 15,21 ----
>   #define TABLECMDS_H
>   
>   #include "nodes/parsenodes.h"
> + #include "utils/inval.h"
>   
>   extern void AlterTableAddColumn(Oid myrelid, bool inherits,
>   								ColumnDef *colDef);
> ***************
> *** 60,63 ****
> --- 61,68 ----
>   extern void renamerel(Oid relid,
>   		  const char *newrelname);
>   
> + extern void renametrig(Oid relid,
> + 		  const char *oldname,
> + 		  const char *newname);
> + 
>   #endif   /* TABLECMDS_H */
> diff -cNr pgsql.cvs.orig/src/include/nodes/parsenodes.h pgsql/src/include/nodes/parsenodes.h
> *** pgsql.cvs.orig/src/include/nodes/parsenodes.h	Fri Apr 19 10:32:55 2002
> --- pgsql/src/include/nodes/parsenodes.h	Fri Apr 19 14:21:21 2002
> ***************
> *** 1205,1221 ****
>   } RemoveOperStmt;
>   
>   /* ----------------------
> !  *		Alter Table Rename Statement
>    * ----------------------
>    */
>   typedef struct RenameStmt
>   {
>   	NodeTag		type;
> ! 	RangeVar   *relation;		/* relation to be altered */
> ! 	char	   *column;			/* if NULL, rename the relation name to
> ! 								 * the new name. Otherwise, rename this
> ! 								 * column name. */
>   	char	   *newname;		/* the new name */
>   } RenameStmt;
>   
>   /* ----------------------
> --- 1205,1227 ----
>   } RemoveOperStmt;
>   
>   /* ----------------------
> !  *		Alter Object Rename Statement
>    * ----------------------
> +  * Currently supports renaming tables, table columns, and triggers.
> +  * If renaming a table, oldname is ignored.
>    */
> + #define RENAME_TABLE	110
> + #define RENAME_COLUMN	111
> + #define RENAME_TRIGGER	112
> + #define RENAME_RULE		113
> + 
>   typedef struct RenameStmt
>   {
>   	NodeTag		type;
> ! 	RangeVar   *relation;		/* owning relation */
> ! 	char	   *oldname;		/* name of rule, trigger, etc */
>   	char	   *newname;		/* the new name */
> + 	int			renameType;		/* RENAME_TABLE, RENAME_COLUMN, etc */
>   } RenameStmt;
>   
>   /* ----------------------

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman(at)candle(dot)pha(dot)pa(dot)us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

In response to

pgsql-hackers by date

Next:From: Joe ConwayDate: 2002-04-23 18:27:52
Subject: Re: Vote on SET in aborted transaction
Previous:From: Bruce MomjianDate: 2002-04-23 17:46:09
Subject: Re: My talk at Linuxtag

pgsql-patches by date

Next:From: Bruce MomjianDate: 2002-04-23 18:35:54
Subject: Re: [PATCHES] patch for ResultSet.java
Previous:From: Bruce MomjianDate: 2002-04-23 16:55:56
Subject: Re: Odd(?) RI-trigger behavior

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group