Thread: RENAME TRIGGER patch (was [HACKERS] Odd(?) RI-trigger behavior)
Tom Lane wrote:
> Joe Conway <mail@joeconway.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;
/* ----------------------
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@joeconway.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@postgresql.org)
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Patch applied. Thanks.
---------------------------------------------------------------------------
Joe Conway wrote:
> Tom Lane wrote:
> > Joe Conway <mail@joeconway.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@postgresql.org)
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026