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

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