From 4c25a0655b9721af5c65922981c03926d856c6e4 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 30 Apr 2002 01:24:57 +0000 Subject: [PATCH] Code review for ALTER TRIGGER RENAME patch: make better use of index, don't scribble on tuple returned by table scan. --- src/backend/commands/trigger.c | 89 +++++++++++++++++++--------------------- src/include/commands/tablecmds.h | 3 +- 2 files changed, 44 insertions(+), 48 deletions(-) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 52e9761d75..a871c85788 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.116 2002/04/27 03:45:02 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.117 2002/04/30 01:24:57 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -514,8 +514,7 @@ RelationRemoveTriggers(Relation rel) * 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 + * update row in catalog */ void renametrig(Oid relid, @@ -526,8 +525,7 @@ renametrig(Oid relid, Relation tgrel; HeapTuple tuple; SysScanDesc tgscan; - ScanKeyData key; - bool found = FALSE; + ScanKeyData key[2]; Relation idescs[Num_pg_trigger_indices]; /* @@ -550,70 +548,69 @@ renametrig(Oid relid, /* * First pass -- look for name conflict */ - ScanKeyEntryInitialize(&key, 0, + ScanKeyEntryInitialize(&key[0], 0, Anum_pg_trigger_tgrelid, F_OIDEQ, ObjectIdGetDatum(relid)); + ScanKeyEntryInitialize(&key[1], 0, + Anum_pg_trigger_tgname, + F_NAMEEQ, + PointerGetDatum(newname)); 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)); - } + SnapshotNow, 2, key); + if (HeapTupleIsValid(tuple = systable_getnext(tgscan))) + 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, + ScanKeyEntryInitialize(&key[0], 0, Anum_pg_trigger_tgrelid, F_OIDEQ, ObjectIdGetDatum(relid)); + ScanKeyEntryInitialize(&key[1], 0, + Anum_pg_trigger_tgname, + F_NAMEEQ, + PointerGetDatum(oldname)); tgscan = systable_beginscan(tgrel, TriggerRelidNameIndex, true, - SnapshotNow, 1, &key); - while (HeapTupleIsValid(tuple = systable_getnext(tgscan))) + SnapshotNow, 2, key); + if (HeapTupleIsValid(tuple = systable_getnext(tgscan))) { - Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(tuple); + /* + * Update pg_trigger tuple with new tgname. + */ + tuple = heap_copytuple(tuple); /* need a modifiable copy */ - 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); + namestrcpy(&((Form_pg_trigger) GETSTRUCT(tuple))->tgname, newname); - /* - * 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); + simple_heap_update(tgrel, &tuple->t_self, tuple); - /* - * 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); + /* + * 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); - found = TRUE; - break; - } + /* + * Invalidate relation's relcache entry so that other backends (and + * this one too!) are sent SI message to make them rebuild relcache + * entries. (Ideally this should happen automatically...) + */ + CacheInvalidateRelcache(relid); } + else + { + elog(ERROR, "renametrig: trigger %s not defined on relation %s", + oldname, RelationGetRelationName(targetrel)); + } + 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! */ diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index 1ce35eb164..646ae45d24 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: tablecmds.h,v 1.3 2002/04/26 19:29:47 tgl Exp $ + * $Id: tablecmds.h,v 1.4 2002/04/30 01:24:52 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -15,7 +15,6 @@ #define TABLECMDS_H #include "nodes/parsenodes.h" -#include "utils/inval.h" extern void AlterTableAddColumn(Oid myrelid, bool inherits, ColumnDef *colDef); -- 2.11.0