From 201737168c4ed5b14313d111d8d746c7f072f24e Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Fri, 19 Apr 2002 16:36:08 +0000 Subject: [PATCH] pg_trigger's index on tgrelid is replaced by a unique index on (tgrelid, tgname). This provides an additional check on trigger name uniqueness per-table (which was already enforced by the code anyway). With this change, RelationBuildTriggers will read the triggers in order by tgname, since it's scanning using this index. Since a predictable trigger ordering has been requested for some time, document this behavior as a feature. Also document that rules fire in name order, since yesterday's changes to pg_rewrite indexing cause that too. --- doc/src/sgml/ref/create_rule.sgml | 40 +++++---- doc/src/sgml/ref/create_trigger.sgml | 40 +++++++-- doc/src/sgml/trigger.sgml | 101 ++++++++++++---------- src/backend/catalog/indexing.c | 4 +- src/backend/commands/comment.c | 23 +++-- src/backend/commands/tablecmds.c | 6 +- src/backend/commands/trigger.c | 86 +++++++++--------- src/backend/utils/cache/relcache.c | 78 ++++++++--------- src/include/catalog/catversion.h | 4 +- src/include/catalog/indexing.h | 8 +- src/test/regress/expected/foreign_key.out | 2 +- 11 files changed, 225 insertions(+), 167 deletions(-) diff --git a/doc/src/sgml/ref/create_rule.sgml b/doc/src/sgml/ref/create_rule.sgml index c9cc209843a..1c5786a0c91 100644 --- a/doc/src/sgml/ref/create_rule.sgml +++ b/doc/src/sgml/ref/create_rule.sgml @@ -1,5 +1,5 @@ <!-- -$Header: /cvsroot/pgsql/doc/src/sgml/ref/create_rule.sgml,v 1.33 2002/03/22 19:20:39 petere Exp $ +$Header: /cvsroot/pgsql/doc/src/sgml/ref/create_rule.sgml,v 1.34 2002/04/19 16:36:08 tgl Exp $ PostgreSQL documentation --> @@ -22,7 +22,7 @@ PostgreSQL documentation </refsynopsisdivinfo> <synopsis> CREATE RULE <replaceable class="parameter">name</replaceable> AS ON <replaceable class="parameter">event</replaceable> - TO <replaceable class="parameter">object</replaceable> [ WHERE <replaceable class="parameter">condition</replaceable> ] + TO <replaceable class="parameter">table</replaceable> [ WHERE <replaceable class="parameter">condition</replaceable> ] DO [ INSTEAD ] <replaceable class="parameter">action</replaceable> where <replaceable class="PARAMETER">action</replaceable> can be: @@ -48,7 +48,8 @@ NOTHING <term><replaceable class="parameter">name</replaceable></term> <listitem> <para> - The name of a rule to create. + The name of a rule to create. This must be distinct from the name + of any other rule for the same table. </para> </listitem> </varlistentry> @@ -63,14 +64,11 @@ NOTHING </listitem> </varlistentry> <varlistentry> - <term><replaceable class="parameter">object</replaceable></term> + <term><replaceable class="parameter">table</replaceable></term> <listitem> <para> - Object is either <replaceable class="parameter">table</replaceable> - or <replaceable class="parameter">table</replaceable>.<replaceable - class="parameter">column</replaceable>. (Currently, only the - <replaceable class="parameter">table</replaceable> form is - actually implemented.) + The name (optionally schema-qualified) of the table or view the rule + applies to. </para> </listitem> </varlistentry> @@ -103,8 +101,7 @@ NOTHING Within the <replaceable class="parameter">condition</replaceable> and <replaceable class="PARAMETER">action</replaceable>, the special table names <literal>new</literal> and <literal>old</literal> may be - used to refer to values in the referenced table (the - <replaceable class="parameter">object</replaceable>). + used to refer to values in the referenced table. <literal>new</literal> is valid in ON INSERT and ON UPDATE rules to refer to the new row being inserted or updated. <literal>old</literal> is valid in ON UPDATE and ON DELETE @@ -159,7 +156,7 @@ CREATE accessed, inserted, updated, or deleted, there is an old instance (for selects, updates and deletes) and a new instance (for inserts and updates). All the rules for the given event type and the given target - object (table) are examined, in an unspecified order. If the + table are examined successively (in order by name). If the <replaceable class="parameter">condition</replaceable> specified in the WHERE clause (if any) is true, the <replaceable class="parameter">action</replaceable> part of the rule is @@ -178,8 +175,7 @@ CREATE The <replaceable class="parameter">action</replaceable> part of the rule can consist of one or more queries. To write multiple queries, surround them with parentheses. Such queries will be performed in the - specified order (whereas there are no guarantees about the execution - order of multiple rules for an object). The <replaceable + specified order. The <replaceable class="parameter">action</replaceable> can also be NOTHING indicating no action. Thus, a DO INSTEAD NOTHING rule suppresses the original query from executing (when its condition is true); a DO NOTHING rule @@ -191,6 +187,20 @@ CREATE executes with the same command and transaction identifier as the user command that caused activation. </para> + + <para> + It is important to realize that a rule is really a query transformation + mechanism, or query macro. The entire query is processed to convert it + into a series of queries that include the rule actions. This occurs + before evaluation of the query starts. So, conditional rules are + handled by adding the rule condition to the WHERE clause of the action(s) + derived from the rule. The above description of a rule as an operation + that executes for each row is thus somewhat misleading. If you actually + want an operation that fires independently for each physical row, you + probably want to use a trigger not a rule. Rules are most useful for + situations that call for transforming entire queries independently of + the specific data being handled. + </para> <refsect2 id="R2-SQL-CREATERULE-3"> <refsect2info> @@ -202,7 +212,7 @@ CREATE <para> Presently, ON SELECT rules must be unconditional INSTEAD rules and must have actions that consist of a single SELECT query. Thus, an ON SELECT - rule effectively turns the object table into a view, whose visible + rule effectively turns the table into a view, whose visible contents are the rows returned by the rule's SELECT query rather than whatever had been stored in the table (if anything). It is considered better style to write a CREATE VIEW command than to create a real table diff --git a/doc/src/sgml/ref/create_trigger.sgml b/doc/src/sgml/ref/create_trigger.sgml index f3e82766380..1d24be9a2b4 100644 --- a/doc/src/sgml/ref/create_trigger.sgml +++ b/doc/src/sgml/ref/create_trigger.sgml @@ -1,5 +1,5 @@ <!-- -$Header: /cvsroot/pgsql/doc/src/sgml/ref/create_trigger.sgml,v 1.22 2002/01/20 22:19:56 petere Exp $ +$Header: /cvsroot/pgsql/doc/src/sgml/ref/create_trigger.sgml,v 1.23 2002/04/19 16:36:08 tgl Exp $ PostgreSQL documentation --> @@ -44,23 +44,24 @@ CREATE TRIGGER <replaceable class="PARAMETER">name</replaceable> { BEFORE | AFTE <term><replaceable class="parameter">name</replaceable></term> <listitem> <para> - The name to give the new trigger. + The name to give the new trigger. This must be distinct from the name + of any other trigger for the same table. </para> </listitem> </varlistentry> <varlistentry> - <term><replaceable class="parameter">table</replaceable></term> + <term><replaceable class="parameter">event</replaceable></term> <listitem> <para> - The name of an existing table. + One of INSERT, DELETE or UPDATE. </para> </listitem> </varlistentry> <varlistentry> - <term><replaceable class="parameter">event</replaceable></term> + <term><replaceable class="parameter">table</replaceable></term> <listitem> <para> - One of INSERT, DELETE or UPDATE. + The name (optionally schema-qualified) of the table the trigger is for. </para> </listitem> </varlistentry> @@ -68,7 +69,20 @@ CREATE TRIGGER <replaceable class="PARAMETER">name</replaceable> { BEFORE | AFTE <term><replaceable class="parameter">func</replaceable></term> <listitem> <para> - A user-supplied function. + A user-supplied function that is declared as taking no arguments + and returning type <literal>opaque</>. + </para> + </listitem> + </varlistentry> + <varlistentry> + <term><replaceable class="parameter">arguments</replaceable></term> + <listitem> + <para> + An optional comma-separated list of arguments to be provided to the + function when the trigger is executed, along with the standard trigger + data such as old and new tuple contents. The arguments are literal + string constants. Simple names and numeric constants may be written + here too, but they will all be converted to strings. </para> </listitem> </varlistentry> @@ -130,6 +144,12 @@ CREATE after the event, all changes, including the last insertion, update, or deletion, are <quote>visible</quote> to the trigger. </para> + + <para> + If multiple triggers of the same kind are defined for the same event, + they will be fired in alphabetical order by name. + </para> + <para> <command>SELECT</command> does not modify any rows so you can not create <command>SELECT</command> triggers. Rules and views are more @@ -262,6 +282,12 @@ CREATE TABLE distributors ( </listitem> </itemizedlist> </para> + + <para> + SQL99 specifies that multiple triggers should be fired in + time-of-creation order. <productname>PostgreSQL</productname> + uses name order, which was judged more convenient to work with. + </para> </listitem> </varlistentry> </variablelist> diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml index 8dd9815e0c6..5456f4d0cdb 100644 --- a/doc/src/sgml/trigger.sgml +++ b/doc/src/sgml/trigger.sgml @@ -1,5 +1,5 @@ <!-- -$Header: /cvsroot/pgsql/doc/src/sgml/trigger.sgml,v 1.22 2002/04/01 22:36:06 tgl Exp $ +$Header: /cvsroot/pgsql/doc/src/sgml/trigger.sgml,v 1.23 2002/04/19 16:36:08 tgl Exp $ --> <chapter id="triggers"> @@ -14,8 +14,8 @@ $Header: /cvsroot/pgsql/doc/src/sgml/trigger.sgml,v 1.22 2002/04/01 22:36:06 tgl AFTER on INSERT, DELETE or UPDATE of a tuple as a trigger event. </para> - <sect1 id="trigger-create"> - <title>Trigger Creation</title> + <sect1 id="trigger-definition"> + <title>Trigger Definition</title> <para> If a trigger event occurs, the trigger manager (called by the Executor) @@ -24,13 +24,17 @@ $Header: /cvsroot/pgsql/doc/src/sgml/trigger.sgml,v 1.22 2002/04/01 22:36:06 tgl </para> <para> - The trigger function must be defined before the trigger is created as a - function taking no arguments and returning opaque. If the function is - written in C, it must use the <quote>version 1</> function manager interface. + The trigger function must be defined before the trigger itself can be + created. The trigger function must be declared as a + function taking no arguments and returning type <literal>opaque</>. + (The trigger function receives its input through a TriggerData + structure, not in the form of ordinary function arguments.) + If the function is written in C, it must use the <quote>version 1</> + function manager interface. </para> <para> - The syntax for creating triggers is as follows: + The syntax for creating triggers is: <programlisting> CREATE TRIGGER <replaceable>trigger</replaceable> [ BEFORE | AFTER ] [ INSERT | DELETE | UPDATE [ OR ... ] ] @@ -48,9 +52,9 @@ CREATE TRIGGER <replaceable>trigger</replaceable> [ BEFORE | AFTER ] [ INSERT | </term> <listitem> <para> - The name of the trigger is - used if you ever have to delete the trigger. - It is used as an argument to the <command>DROP TRIGGER</command> command. + The trigger must have a name distinct from all other triggers on + the same table. The name is needed + if you ever have to delete the trigger. </para> </listitem> </varlistentry> @@ -72,7 +76,7 @@ CREATE TRIGGER <replaceable>trigger</replaceable> [ BEFORE | AFTER ] [ INSERT | <term>UPDATE</term> <listitem> <para> - The next element of the command determines on what event(s) will trigger + The next element of the command determines what event(s) will trigger the function. Multiple events can be specified separated by OR. </para> </listitem> @@ -82,7 +86,7 @@ CREATE TRIGGER <replaceable>trigger</replaceable> [ BEFORE | AFTER ] [ INSERT | <term><replaceable>relation</replaceable></term> <listitem> <para> - The relation name determines which table the event applies to. + The relation name indicates which table the event applies to. </para> </listitem> </varlistentry> @@ -94,6 +98,7 @@ CREATE TRIGGER <replaceable>trigger</replaceable> [ BEFORE | AFTER ] [ INSERT | <para> The FOR EACH clause determines whether the trigger is fired for each affected row or before (or after) the entire statement has completed. + Currently only the ROW case is supported. </para> </listitem> </varlistentry> @@ -102,7 +107,7 @@ CREATE TRIGGER <replaceable>trigger</replaceable> [ BEFORE | AFTER ] [ INSERT | <term><replaceable>procedure</replaceable></term> <listitem> <para> - The procedure name is the function called. + The procedure name is the function to be called. </para> </listitem> </varlistentry> @@ -112,23 +117,23 @@ CREATE TRIGGER <replaceable>trigger</replaceable> [ BEFORE | AFTER ] [ INSERT | <listitem> <para> The arguments passed to the function in the TriggerData structure. - The purpose of passing arguments to the function is to allow different - triggers with similar requirements to call the same function. + This is either empty or a list of one or more simple literal + constants (which will be passed to the function as strings). </para> <para> - Also, <replaceable>procedure</replaceable> - may be used for triggering different relations (these - functions are named as <firstterm>general trigger functions</>). - </para> - - <para> - As example of using both features above, there could be a general - function that takes as its arguments two field names and puts the current - user in one and the current timestamp in the other. This allows triggers to - be written on INSERT events to automatically track creation of records in a - transaction table for example. It could also be used as a <quote>last updated</> - function if used in an UPDATE event. + The purpose of including arguments in the trigger definition + is to allow different + triggers with similar requirements to call the same function. + As an example, there could be a generalized trigger + function that takes as its arguments two field names and puts the + current user in one and the current timestamp in the other. + Properly written, this trigger function would be independent of + the specific table it is triggering on. So the same function + could be used for INSERT events on any table with suitable fields, + to automatically track creation of records in a transaction table for + example. It could also be used to track last-update events if + defined as an UPDATE trigger. </para> </listitem> </varlistentry> @@ -136,8 +141,8 @@ CREATE TRIGGER <replaceable>trigger</replaceable> [ BEFORE | AFTER ] [ INSERT | </para> <para> - Trigger functions return HeapTuple to the calling Executor. This - is ignored for triggers fired after an INSERT, DELETE or UPDATE operation + Trigger functions return a HeapTuple to the calling Executor. The return + value is ignored for triggers fired AFTER an operation, but it allows BEFORE triggers to: <itemizedlist> @@ -150,33 +155,41 @@ CREATE TRIGGER <replaceable>trigger</replaceable> [ BEFORE | AFTER ] [ INSERT | <listitem> <para> - Return a pointer to another tuple (INSERT and UPDATE only) which will - be inserted (as the new version of the updated tuple if UPDATE) instead - of original tuple. + For INSERT and UPDATE triggers only, the returned tuple becomes the + tuple which will be inserted or will replace the tuple being updated. + This allows the trigger function to modify the row being inserted or + updated. </para> </listitem> </itemizedlist> + + A BEFORE trigger that does not intend to cause either of these behaviors + must be careful to return the same NEW tuple it is passed. </para> <para> Note that there is no initialization performed by the CREATE TRIGGER - handler. This will be changed in the future. Also, if more than one trigger - is defined for the same event on the same relation, the order of trigger - firing is unpredictable. This may be changed in the future. + handler. This may be changed in the future. </para> <para> - If a trigger function executes SQL-queries (using SPI) then these queries - may fire triggers again. This is known as cascading triggers. There is no - explicit limitation on the number of cascade levels. + If more than one trigger + is defined for the same event on the same relation, the triggers will + be fired in alphabetical order by name. In the case of BEFORE triggers, + the possibly-modified tuple returned by each trigger becomes the input + to the next trigger. If any BEFORE trigger returns NULL, the operation + is abandoned and subsequent triggers are not fired. </para> <para> - If a trigger is fired by INSERT and inserts a new tuple in the same - relation then this trigger will be fired again. Currently, there is nothing - provided for synchronization (etc) of these cases but this may change. At - the moment, there is function funny_dup17() in the regress tests which uses - some techniques to stop recursion (cascading) on itself... + If a trigger function executes SQL-queries (using SPI) then these queries + may fire triggers again. This is known as cascading triggers. There is no + direct limitation on the number of cascade levels. It is possible for + cascades to cause recursive invocation of the same trigger --- for + example, an INSERT trigger might execute a query that inserts an + additional tuple into the same table, causing the INSERT trigger to be + fired again. It is the trigger programmer's + responsibility to avoid infinite recursion in such scenarios. </para> </sect1> @@ -326,7 +339,7 @@ typedef struct TriggerData <para> is a pointer to structure describing the triggered relation. Look at src/include/utils/rel.h for details about this structure. The most - interest things are tg_relation->rd_att (descriptor of the relation + interesting things are tg_relation->rd_att (descriptor of the relation tuples) and tg_relation->rd_rel->relname (relation's name. This is not char*, but NameData. Use SPI_getrelname(tg_relation) to get char* if you need a copy of name). diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c index 6ccb1ef0b2b..cc26af128ae 100644 --- a/src/backend/catalog/indexing.c +++ b/src/backend/catalog/indexing.c @@ -9,7 +9,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/catalog/indexing.c,v 1.91 2002/04/18 20:01:09 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/catalog/indexing.c,v 1.92 2002/04/19 16:36:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -74,7 +74,7 @@ char *Name_pg_shadow_indices[Num_pg_shadow_indices] = char *Name_pg_statistic_indices[Num_pg_statistic_indices] = {StatisticRelidAttnumIndex}; char *Name_pg_trigger_indices[Num_pg_trigger_indices] = -{TriggerRelidIndex, TriggerConstrNameIndex, TriggerConstrRelidIndex, TriggerOidIndex}; +{TriggerRelidNameIndex, TriggerConstrNameIndex, TriggerConstrRelidIndex, TriggerOidIndex}; char *Name_pg_type_indices[Num_pg_type_indices] = {TypeNameNspIndex, TypeOidIndex}; char *Name_pg_description_indices[Num_pg_description_indices] = diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c index 07b14ca4fe2..63c023cf0e2 100644 --- a/src/backend/commands/comment.c +++ b/src/backend/commands/comment.c @@ -7,7 +7,7 @@ * Copyright (c) 1999-2001, PostgreSQL Global Development Group * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/comment.c,v 1.42 2002/04/18 20:01:09 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/comment.c,v 1.43 2002/04/19 16:36:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -753,7 +753,7 @@ CommentTrigger(List *qualname, char *comment) Relation pg_trigger, relation; HeapTuple triggertuple; - HeapScanDesc scan; + SysScanDesc scan; ScanKeyData entry[2]; Oid oid; @@ -774,17 +774,22 @@ CommentTrigger(List *qualname, char *comment) elog(ERROR, "you are not permitted to comment on trigger '%s' for relation '%s'", trigname, RelationGetRelationName(relation)); - /* Fetch the trigger oid from pg_trigger */ - + /* + * Fetch the trigger tuple from pg_trigger. There can be only one + * because of the unique index. + */ pg_trigger = heap_openr(TriggerRelationName, AccessShareLock); - ScanKeyEntryInitialize(&entry[0], 0x0, Anum_pg_trigger_tgrelid, + ScanKeyEntryInitialize(&entry[0], 0x0, + Anum_pg_trigger_tgrelid, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(relation))); - ScanKeyEntryInitialize(&entry[1], 0x0, Anum_pg_trigger_tgname, + ScanKeyEntryInitialize(&entry[1], 0x0, + Anum_pg_trigger_tgname, F_NAMEEQ, CStringGetDatum(trigname)); - scan = heap_beginscan(pg_trigger, 0, SnapshotNow, 2, entry); - triggertuple = heap_getnext(scan, 0); + scan = systable_beginscan(pg_trigger, TriggerRelidNameIndex, true, + SnapshotNow, 2, entry); + triggertuple = systable_getnext(scan); /* If no trigger exists for the relation specified, notify user */ @@ -794,7 +799,7 @@ CommentTrigger(List *qualname, char *comment) oid = triggertuple->t_data->t_oid; - heap_endscan(scan); + systable_endscan(scan); /* Create the comments with the pg_trigger oid */ diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 8abd8cf80de..b0b73af76e4 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v 1.3 2002/04/18 20:01:09 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v 1.4 2002/04/19 16:36:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -2909,10 +2909,10 @@ update_ri_trigger_args(Oid relid, if (fk_scan) irel = index_openr(TriggerConstrRelidIndex); else - irel = index_openr(TriggerRelidIndex); + irel = index_openr(TriggerRelidNameIndex); ScanKeyEntryInitialize(&skey[0], 0x0, - 1, /* always column 1 of index */ + 1, /* column 1 of index in either case */ F_OIDEQ, ObjectIdGetDatum(relid)); idxtgscan = index_beginscan(irel, false, 1, skey); diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 891b9f3cfba..c00b4bebac8 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.113 2002/04/12 20:38:24 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.114 2002/04/19 16:36:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -90,14 +90,15 @@ CreateTrigger(CreateTrigStmt *stmt) elog(ERROR, "permission denied"); /* - * If trigger is a constraint, user trigger name as constraint name + * If trigger is an RI constraint, use trigger name as constraint name * and build a unique trigger name instead. */ if (stmt->isconstraint) { constrname = stmt->trigname; + snprintf(constrtrigname, sizeof(constrtrigname), + "RI_ConstraintTrigger_%u", newoid()); stmt->trigname = constrtrigname; - sprintf(constrtrigname, "RI_ConstraintTrigger_%u", newoid()); if (stmt->constrrel != NULL) constrrelid = RangeVarGetRelid(stmt->constrrel, false); @@ -139,15 +140,20 @@ CreateTrigger(CreateTrigStmt *stmt) } /* - * Scan pg_trigger for existing triggers on relation. NOTE that this - * is cool only because we have AccessExclusiveLock on the relation, - * so the trigger set won't be changing underneath us. + * Scan pg_trigger for existing triggers on relation. We do this mainly + * because we must count them; a secondary benefit is to give a nice + * error message if there's already a trigger of the same name. (The + * unique index on tgrelid/tgname would complain anyway.) + * + * 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); - ScanKeyEntryInitialize(&key, 0, Anum_pg_trigger_tgrelid, + ScanKeyEntryInitialize(&key, 0, + Anum_pg_trigger_tgrelid, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(rel))); - tgscan = systable_beginscan(tgrel, TriggerRelidIndex, true, + tgscan = systable_beginscan(tgrel, TriggerRelidNameIndex, true, SnapshotNow, 1, &key); while (HeapTupleIsValid(tuple = systable_getnext(tgscan))) { @@ -336,15 +342,20 @@ DropTrigger(Oid relid, const char *trigname) /* * Search pg_trigger, delete target trigger, count remaining triggers - * for relation. Note this is OK only because we have - * AccessExclusiveLock on the rel, so no one else is creating/deleting - * triggers on this rel at the same time. + * for relation. (Although we could fetch and delete the target + * trigger directly, we'd still have to scan the remaining triggers, + * so we may as well do both in one indexscan.) + * + * Note this is OK only because we have AccessExclusiveLock on the rel, + * so no one else is creating/deleting triggers on this rel at the same + * time. */ tgrel = heap_openr(TriggerRelationName, RowExclusiveLock); - ScanKeyEntryInitialize(&key, 0, Anum_pg_trigger_tgrelid, + ScanKeyEntryInitialize(&key, 0, + Anum_pg_trigger_tgrelid, F_OIDEQ, ObjectIdGetDatum(relid)); - tgscan = systable_beginscan(tgrel, TriggerRelidIndex, true, + tgscan = systable_beginscan(tgrel, TriggerRelidNameIndex, true, SnapshotNow, 1, &key); while (HeapTupleIsValid(tuple = systable_getnext(tgscan))) { @@ -409,10 +420,11 @@ RelationRemoveTriggers(Relation rel) bool found = false; tgrel = heap_openr(TriggerRelationName, RowExclusiveLock); - ScanKeyEntryInitialize(&key, 0, Anum_pg_trigger_tgrelid, + ScanKeyEntryInitialize(&key, 0, + Anum_pg_trigger_tgrelid, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(rel))); - tgscan = systable_beginscan(tgrel, TriggerRelidIndex, true, + tgscan = systable_beginscan(tgrel, TriggerRelidNameIndex, true, SnapshotNow, 1, &key); while (HeapTupleIsValid(tup = systable_getnext(tgscan))) @@ -462,7 +474,8 @@ RelationRemoveTriggers(Relation rel) /* * Also drop all constraint triggers referencing this relation */ - ScanKeyEntryInitialize(&key, 0, Anum_pg_trigger_tgconstrrelid, + ScanKeyEntryInitialize(&key, 0, + Anum_pg_trigger_tgconstrrelid, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(rel))); tgscan = systable_beginscan(tgrel, TriggerConstrRelidIndex, true, @@ -502,7 +515,7 @@ RelationBuildTriggers(Relation relation) { TriggerDesc *trigdesc; int ntrigs = relation->rd_rel->reltriggers; - Trigger *triggers = NULL; + Trigger *triggers; int found = 0; Relation tgrel; ScanKeyData skey; @@ -511,6 +524,15 @@ RelationBuildTriggers(Relation relation) struct varlena *val; bool isnull; + triggers = (Trigger *) MemoryContextAlloc(CacheMemoryContext, + ntrigs * sizeof(Trigger)); + + /* + * Note: since we scan the triggers using TriggerRelidNameIndex, + * we will be reading the triggers in name order, except possibly + * during emergency-recovery operations (ie, IsIgnoringSystemIndexes). + * This in turn ensures that triggers will be fired in name order. + */ ScanKeyEntryInitialize(&skey, (bits16) 0x0, (AttrNumber) Anum_pg_trigger_tgrelid, @@ -518,7 +540,7 @@ RelationBuildTriggers(Relation relation) ObjectIdGetDatum(RelationGetRelid(relation))); tgrel = heap_openr(TriggerRelationName, AccessShareLock); - tgscan = systable_beginscan(tgrel, TriggerRelidIndex, true, + tgscan = systable_beginscan(tgrel, TriggerRelidNameIndex, true, SnapshotNow, 1, &skey); while (HeapTupleIsValid(htup = systable_getnext(tgscan))) @@ -526,16 +548,9 @@ RelationBuildTriggers(Relation relation) Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(htup); Trigger *build; - if (found == ntrigs) + if (found >= ntrigs) elog(ERROR, "RelationBuildTriggers: unexpected record found for rel %s", RelationGetRelationName(relation)); - - if (triggers == NULL) - triggers = (Trigger *) MemoryContextAlloc(CacheMemoryContext, - sizeof(Trigger)); - else - triggers = (Trigger *) repalloc(triggers, - (found + 1) * sizeof(Trigger)); build = &(triggers[found]); build->tgoid = htup->t_data->t_oid; @@ -730,6 +745,9 @@ equalTriggerDescs(TriggerDesc *trigdesc1, TriggerDesc *trigdesc2) * We need not examine the "index" data, just the trigger array * itself; if we have the same triggers with the same types, the * derived index data should match. + * + * As of 7.3 we assume trigger set ordering is significant in the + * comparison; so we just compare corresponding slots of the two sets. */ if (trigdesc1 != NULL) { @@ -740,21 +758,9 @@ equalTriggerDescs(TriggerDesc *trigdesc1, TriggerDesc *trigdesc2) for (i = 0; i < trigdesc1->numtriggers; i++) { Trigger *trig1 = trigdesc1->triggers + i; - Trigger *trig2 = NULL; + Trigger *trig2 = trigdesc2->triggers + i; - /* - * We can't assume that the triggers are always read from - * pg_trigger in the same order; so use the trigger OIDs to - * identify the triggers to compare. (We assume here that the - * same OID won't appear twice in either trigger set.) - */ - for (j = 0; j < trigdesc2->numtriggers; j++) - { - trig2 = trigdesc2->triggers + j; - if (trig1->tgoid == trig2->tgoid) - break; - } - if (j >= trigdesc2->numtriggers) + if (trig1->tgoid != trig2->tgoid) return false; if (strcmp(trig1->tgname, trig2->tgname) != 0) return false; diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 686fcc8416b..983055ef783 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.161 2002/04/18 20:01:09 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.162 2002/04/19 16:36:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -635,10 +635,10 @@ RelationBuildRuleLock(Relation relation) { MemoryContext rulescxt; MemoryContext oldcxt; - HeapTuple pg_rewrite_tuple; - Relation pg_rewrite_desc; - TupleDesc pg_rewrite_tupdesc; - SysScanDesc pg_rewrite_scan; + HeapTuple rewrite_tuple; + Relation rewrite_desc; + TupleDesc rewrite_tupdesc; + SysScanDesc rewrite_scan; ScanKeyData key; RuleLock *rulelock; int numlocks; @@ -657,7 +657,7 @@ RelationBuildRuleLock(Relation relation) relation->rd_rulescxt = rulescxt; /* - * form an array to hold the rewrite rules (the array is extended if + * allocate an array to hold the rewrite rules (the array is extended if * necessary) */ maxlocks = 4; @@ -675,18 +675,22 @@ RelationBuildRuleLock(Relation relation) /* * open pg_rewrite and begin a scan - */ - pg_rewrite_desc = heap_openr(RewriteRelationName, AccessShareLock); - pg_rewrite_tupdesc = RelationGetDescr(pg_rewrite_desc); - pg_rewrite_scan = systable_beginscan(pg_rewrite_desc, - RewriteRelRulenameIndex, - criticalRelcachesBuilt, - SnapshotNow, - 1, &key); - - while (HeapTupleIsValid(pg_rewrite_tuple = systable_getnext(pg_rewrite_scan))) + * + * Note: since we scan the rules using RewriteRelRulenameIndex, + * we will be reading the rules in name order, except possibly + * during emergency-recovery operations (ie, IsIgnoringSystemIndexes). + * This in turn ensures that rules will be fired in name order. + */ + rewrite_desc = heap_openr(RewriteRelationName, AccessShareLock); + rewrite_tupdesc = RelationGetDescr(rewrite_desc); + rewrite_scan = systable_beginscan(rewrite_desc, + RewriteRelRulenameIndex, + true, SnapshotNow, + 1, &key); + + while (HeapTupleIsValid(rewrite_tuple = systable_getnext(rewrite_scan))) { - Form_pg_rewrite rewrite_form = (Form_pg_rewrite) GETSTRUCT(pg_rewrite_tuple); + Form_pg_rewrite rewrite_form = (Form_pg_rewrite) GETSTRUCT(rewrite_tuple); bool isnull; Datum ruleaction; Datum rule_evqual; @@ -697,7 +701,7 @@ RelationBuildRuleLock(Relation relation) rule = (RewriteRule *) MemoryContextAlloc(rulescxt, sizeof(RewriteRule)); - rule->ruleId = pg_rewrite_tuple->t_data->t_oid; + rule->ruleId = rewrite_tuple->t_data->t_oid; rule->event = rewrite_form->ev_type - '0'; rule->attrno = rewrite_form->ev_attr; @@ -705,9 +709,9 @@ RelationBuildRuleLock(Relation relation) /* Must use heap_getattr to fetch ev_qual and ev_action */ - ruleaction = heap_getattr(pg_rewrite_tuple, + ruleaction = heap_getattr(rewrite_tuple, Anum_pg_rewrite_ev_action, - pg_rewrite_tupdesc, + rewrite_tupdesc, &isnull); Assert(!isnull); ruleaction_str = DatumGetCString(DirectFunctionCall1(textout, @@ -717,13 +721,13 @@ RelationBuildRuleLock(Relation relation) MemoryContextSwitchTo(oldcxt); pfree(ruleaction_str); - rule_evqual = heap_getattr(pg_rewrite_tuple, + rule_evqual = heap_getattr(rewrite_tuple, Anum_pg_rewrite_ev_qual, - pg_rewrite_tupdesc, + rewrite_tupdesc, &isnull); Assert(!isnull); rule_evqual_str = DatumGetCString(DirectFunctionCall1(textout, - rule_evqual)); + rule_evqual)); oldcxt = MemoryContextSwitchTo(rulescxt); rule->qual = (Node *) stringToNode(rule_evqual_str); MemoryContextSwitchTo(oldcxt); @@ -741,8 +745,8 @@ RelationBuildRuleLock(Relation relation) /* * end the scan and close the attribute relation */ - systable_endscan(pg_rewrite_scan); - heap_close(pg_rewrite_desc, AccessShareLock); + systable_endscan(rewrite_scan); + heap_close(rewrite_desc, AccessShareLock); /* * form a RuleLock and insert into relation @@ -764,9 +768,13 @@ RelationBuildRuleLock(Relation relation) static bool equalRuleLocks(RuleLock *rlock1, RuleLock *rlock2) { - int i, - j; + int i; + /* + * As of 7.3 we assume the rule ordering is repeatable, + * because RelationBuildRuleLock should read 'em in a + * consistent order. So just compare corresponding slots. + */ if (rlock1 != NULL) { if (rlock2 == NULL) @@ -776,21 +784,9 @@ equalRuleLocks(RuleLock *rlock1, RuleLock *rlock2) for (i = 0; i < rlock1->numLocks; i++) { RewriteRule *rule1 = rlock1->rules[i]; - RewriteRule *rule2 = NULL; + RewriteRule *rule2 = rlock2->rules[i]; - /* - * We can't assume that the rules are always read from - * pg_rewrite in the same order; so use the rule OIDs to - * identify the rules to compare. (We assume here that the - * same OID won't appear twice in either ruleset.) - */ - for (j = 0; j < rlock2->numLocks; j++) - { - rule2 = rlock2->rules[j]; - if (rule1->ruleId == rule2->ruleId) - break; - } - if (j >= rlock2->numLocks) + if (rule1->ruleId != rule2->ruleId) return false; if (rule1->event != rule2->event) return false; diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 4d6742fdbd0..73ecb7fc6bd 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -37,7 +37,7 @@ * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: catversion.h,v 1.118 2002/04/18 20:01:10 tgl Exp $ + * $Id: catversion.h,v 1.119 2002/04/19 16:36:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 200204181 +#define CATALOG_VERSION_NO 200204182 #endif diff --git a/src/include/catalog/indexing.h b/src/include/catalog/indexing.h index e08e3aa8e96..0ee35e3525f 100644 --- a/src/include/catalog/indexing.h +++ b/src/include/catalog/indexing.h @@ -8,7 +8,7 @@ * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: indexing.h,v 1.65 2002/04/18 20:01:10 tgl Exp $ + * $Id: indexing.h,v 1.66 2002/04/19 16:36:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -86,7 +86,7 @@ #define StatisticRelidAttnumIndex "pg_statistic_relid_att_index" #define TriggerConstrNameIndex "pg_trigger_tgconstrname_index" #define TriggerConstrRelidIndex "pg_trigger_tgconstrrelid_index" -#define TriggerRelidIndex "pg_trigger_tgrelid_index" +#define TriggerRelidNameIndex "pg_trigger_tgrelid_tgname_index" #define TriggerOidIndex "pg_trigger_oid_index" #define TypeNameNspIndex "pg_type_typname_nsp_index" #define TypeOidIndex "pg_type_oid_index" @@ -182,9 +182,11 @@ DECLARE_UNIQUE_INDEX(pg_rewrite_rel_rulename_index on pg_rewrite using btree(ev_ DECLARE_UNIQUE_INDEX(pg_shadow_usename_index on pg_shadow using btree(usename name_ops)); DECLARE_UNIQUE_INDEX(pg_shadow_usesysid_index on pg_shadow using btree(usesysid int4_ops)); DECLARE_UNIQUE_INDEX(pg_statistic_relid_att_index on pg_statistic using btree(starelid oid_ops, staattnum int2_ops)); +/* This following index is not used for a cache and is not unique */ DECLARE_INDEX(pg_trigger_tgconstrname_index on pg_trigger using btree(tgconstrname name_ops)); +/* This following index is not used for a cache and is not unique */ DECLARE_INDEX(pg_trigger_tgconstrrelid_index on pg_trigger using btree(tgconstrrelid oid_ops)); -DECLARE_INDEX(pg_trigger_tgrelid_index on pg_trigger using btree(tgrelid oid_ops)); +DECLARE_UNIQUE_INDEX(pg_trigger_tgrelid_tgname_index on pg_trigger using btree(tgrelid oid_ops, tgname name_ops)); DECLARE_UNIQUE_INDEX(pg_trigger_oid_index on pg_trigger using btree(oid oid_ops)); DECLARE_UNIQUE_INDEX(pg_type_oid_index on pg_type using btree(oid oid_ops)); DECLARE_UNIQUE_INDEX(pg_type_typname_nsp_index on pg_type using btree(typname name_ops, typnamespace oid_ops)); diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 0d0b6a6cb83..7f4c5f8561e 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -899,7 +899,7 @@ delete from pktable where base1=2; ERROR: <unnamed> referential integrity violation - key in pktable still referenced from pktable -- fails (1,1) is being referenced (twice) update pktable set base1=3 where base1=1; -ERROR: <unnamed> referential integrity violation - key in pktable still referenced from pktable +ERROR: <unnamed> referential integrity violation - key referenced from pktable not found in pktable -- this sequence of two deletes will work, since after the first there will be no (2,*) references delete from pktable where base2=2; delete from pktable where base1=2; -- GitLab