TG_DEPTH patch v1
The people who write my paychecks have insisted on me chunking out
some items which are part of our long-term plan besides the one I've
been focusing on lately. Most of it isn't of interest to anyone
outside Wisconsin Courts, but this piece might be; so I'm posting it
and putting onto the first 9.2 CF. We'll be using it for
development starting Monday and in production in two or three
months, so it should be pretty well tested by July. :-)
-Kevin
Attachments:
depth-1.patchtext/plain; name=depth-1.patchDownload
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***************
*** 3207,3212 **** RAISE unique_violation USING MESSAGE = 'Duplicate user ID: ' || user_id;
--- 3207,3222 ----
</varlistentry>
<varlistentry>
+ <term><varname>TG_DEPTH</varname></term>
+ <listitem>
+ <para>
+ Data type <type>integer</type>; the current number of levels of
+ nesting within trigger execution.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><varname>TG_NARGS</varname></term>
<listitem>
<para>
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
***************
*** 142,147 **** typedef struct TransactionStateData
--- 142,148 ----
Oid prevUser; /* previous CurrentUserId setting */
int prevSecContext; /* previous SecurityRestrictionContext */
bool prevXactReadOnly; /* entry-time xact r/o state */
+ int prevTgDepth; /* previous trigger depth */
bool startedInRecovery; /* did we start in recovery? */
struct TransactionStateData *parent; /* back link to parent */
} TransactionStateData;
***************
*** 171,176 **** static TransactionStateData TopTransactionStateData = {
--- 172,178 ----
InvalidOid, /* previous CurrentUserId setting */
0, /* previous SecurityRestrictionContext */
false, /* entry-time xact r/o state */
+ 0, /* previous trigger depth */
false, /* startedInRecovery */
NULL /* link to parent state block */
};
***************
*** 3996,4001 **** CommitSubTransaction(void)
--- 3998,4005 ----
*/
XactReadOnly = s->prevXactReadOnly;
+ tg_depth = s->prevTgDepth;
+
CurrentResourceOwner = s->parent->curTransactionOwner;
CurTransactionResourceOwner = s->parent->curTransactionOwner;
ResourceOwnerDelete(s->curTransactionOwner);
***************
*** 4114,4119 **** AbortSubTransaction(void)
--- 4118,4125 ----
*/
XactReadOnly = s->prevXactReadOnly;
+ tg_depth = s->prevTgDepth;
+
RESUME_INTERRUPTS();
}
***************
*** 4196,4201 **** PushTransaction(void)
--- 4202,4208 ----
s->blockState = TBLOCK_SUBBEGIN;
GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext);
s->prevXactReadOnly = XactReadOnly;
+ s->prevTgDepth = tg_depth;
CurrentTransactionState = s;
*** a/src/backend/commands/trigger.c
--- b/src/backend/commands/trigger.c
***************
*** 55,60 ****
--- 55,63 ----
#include "utils/tqual.h"
+ /* How many levels deep into trigger execution are we? */
+ int tg_depth = 0;
+
/* GUC variables */
int SessionReplicationRole = SESSION_REPLICATION_ROLE_ORIGIN;
***************
*** 1807,1812 **** ExecCallTriggerFunc(TriggerData *trigdata,
--- 1810,1817 ----
if (instr)
InstrStartNode(instr + tgindx);
+ tg_depth++;
+
/*
* Do the function evaluation in the per-tuple memory context, so that
* leaked memory will be reclaimed once per tuple. Note in particular that
***************
*** 1828,1833 **** ExecCallTriggerFunc(TriggerData *trigdata,
--- 1833,1840 ----
MemoryContextSwitchTo(oldContext);
+ tg_depth--;
+
/*
* Trigger protocol allows function to return a null pointer, but NOT to
* set the isnull result flag.
*** a/src/backend/tcop/pquery.c
--- b/src/backend/tcop/pquery.c
***************
*** 748,753 **** PortalRun(Portal portal, long count, bool isTopLevel,
--- 748,755 ----
errmsg("portal \"%s\" cannot be run", portal->name)));
portal->status = PORTAL_ACTIVE;
+ tg_depth = 0;
+
/*
* Set up global portal context pointers.
*
***************
*** 1371,1376 **** PortalRunFetch(Portal portal,
--- 1373,1380 ----
errmsg("portal \"%s\" cannot be run", portal->name)));
portal->status = PORTAL_ACTIVE;
+ tg_depth = 0;
+
/*
* Set up global portal context pointers.
*/
*** a/src/include/commands/trigger.h
--- b/src/include/commands/trigger.h
***************
*** 108,113 **** extern PGDLLIMPORT int SessionReplicationRole;
--- 108,115 ----
#define TRIGGER_FIRES_ON_REPLICA 'R'
#define TRIGGER_DISABLED 'D'
+ extern int tg_depth;
+
extern Oid CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
Oid constraintOid, Oid indexOid,
bool isInternal);
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
***************
*** 633,638 **** do_compile(FunctionCallInfo fcinfo,
--- 633,644 ----
true);
function->tg_table_schema_varno = var->dno;
+ /* add the variable tg_depth */
+ var = plpgsql_build_variable("tg_depth", 0,
+ plpgsql_build_datatype(INT4OID, -1),
+ true);
+ function->tg_depth_varno = var->dno;
+
/* Add the variable tg_nargs */
var = plpgsql_build_variable("tg_nargs", 0,
plpgsql_build_datatype(INT4OID, -1),
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***************
*** 625,630 **** plpgsql_exec_trigger(PLpgSQL_function *func,
--- 625,635 ----
var->isnull = false;
var->freeval = true;
+ var = (PLpgSQL_var *) (estate.datums[func->tg_depth_varno]);
+ var->value = Int16GetDatum(tg_depth);
+ var->isnull = false;
+ var->freeval = false;
+
var = (PLpgSQL_var *) (estate.datums[func->tg_nargs_varno]);
var->value = Int16GetDatum(trigdata->tg_trigger->tgnargs);
var->isnull = false;
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
***************
*** 668,673 **** typedef struct PLpgSQL_function
--- 668,674 ----
int tg_relname_varno;
int tg_table_name_varno;
int tg_table_schema_varno;
+ int tg_depth_varno;
int tg_nargs_varno;
int tg_argv_varno;
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
***************
*** 4240,4242 **** select unreserved_test();
--- 4240,4317 ----
(1 row)
drop function unreserved_test();
+ -- Test TG_DEPTH
+ create table tg_depth_a (id int not null primary key);
+ NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "tg_depth_a_pkey" for table "tg_depth_a"
+ create table tg_depth_b (id int not null primary key);
+ NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "tg_depth_b_pkey" for table "tg_depth_b"
+ create table tg_depth_c (id int not null primary key);
+ NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "tg_depth_c_pkey" for table "tg_depth_c"
+ create function tg_depth_a_tf() returns trigger
+ language plpgsql as $$
+ begin
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ insert into tg_depth_b values (new.id);
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ return new;
+ end;
+ $$;
+ create trigger tg_depth_a_tr before insert on tg_depth_a
+ for each row execute procedure tg_depth_a_tf();
+ create function tg_depth_b_tf() returns trigger
+ language plpgsql as $$
+ begin
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ begin
+ execute 'insert into tg_depth_c values (' || new.id::text || ')';
+ exception
+ when sqlstate 'U9999' then
+ raise notice 'SQLSTATE = U9999: tg_depth = %', tg_depth;
+ end;
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ execute 'insert into tg_depth_c values (' || new.id::text || ')';
+ return new;
+ end;
+ $$;
+ create trigger tg_depth_b_tr before insert on tg_depth_b
+ for each row execute procedure tg_depth_b_tf();
+ create function tg_depth_c_tf() returns trigger
+ language plpgsql as $$
+ begin
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ raise exception sqlstate 'U9999';
+ return new;
+ end;
+ $$;
+ create trigger tg_depth_c_tr before insert on tg_depth_c
+ for each row execute procedure tg_depth_c_tf();
+ insert into tg_depth_a values (999);
+ NOTICE: tg_depth_a_tr: tg_depth = 1
+ NOTICE: tg_depth_b_tr: tg_depth = 2
+ CONTEXT: SQL statement "insert into tg_depth_b values (new.id)"
+ PL/pgSQL function "tg_depth_a_tf" line 4 at SQL statement
+ NOTICE: tg_depth_c_tr: tg_depth = 3
+ CONTEXT: SQL statement "insert into tg_depth_c values (999)"
+ PL/pgSQL function "tg_depth_b_tf" line 5 at EXECUTE statement
+ SQL statement "insert into tg_depth_b values (new.id)"
+ PL/pgSQL function "tg_depth_a_tf" line 4 at SQL statement
+ NOTICE: SQLSTATE = U9999: tg_depth = 2
+ CONTEXT: SQL statement "insert into tg_depth_b values (new.id)"
+ PL/pgSQL function "tg_depth_a_tf" line 4 at SQL statement
+ NOTICE: tg_depth_b_tr: tg_depth = 2
+ CONTEXT: SQL statement "insert into tg_depth_b values (new.id)"
+ PL/pgSQL function "tg_depth_a_tf" line 4 at SQL statement
+ NOTICE: tg_depth_c_tr: tg_depth = 3
+ CONTEXT: SQL statement "insert into tg_depth_c values (999)"
+ PL/pgSQL function "tg_depth_b_tf" line 11 at EXECUTE statement
+ SQL statement "insert into tg_depth_b values (new.id)"
+ PL/pgSQL function "tg_depth_a_tf" line 4 at SQL statement
+ ERROR: U9999
+ CONTEXT: SQL statement "insert into tg_depth_c values (999)"
+ PL/pgSQL function "tg_depth_b_tf" line 11 at EXECUTE statement
+ SQL statement "insert into tg_depth_b values (new.id)"
+ PL/pgSQL function "tg_depth_a_tf" line 4 at SQL statement
+ drop table tg_depth_a, tg_depth_b, tg_depth_c;
+ drop function tg_depth_a_tf();
+ drop function tg_depth_b_tf();
+ drop function tg_depth_c_tf();
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
***************
*** 3375,3377 **** $$ language plpgsql;
--- 3375,3431 ----
select unreserved_test();
drop function unreserved_test();
+
+ -- Test TG_DEPTH
+
+ create table tg_depth_a (id int not null primary key);
+ create table tg_depth_b (id int not null primary key);
+ create table tg_depth_c (id int not null primary key);
+
+ create function tg_depth_a_tf() returns trigger
+ language plpgsql as $$
+ begin
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ insert into tg_depth_b values (new.id);
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ return new;
+ end;
+ $$;
+ create trigger tg_depth_a_tr before insert on tg_depth_a
+ for each row execute procedure tg_depth_a_tf();
+
+ create function tg_depth_b_tf() returns trigger
+ language plpgsql as $$
+ begin
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ begin
+ execute 'insert into tg_depth_c values (' || new.id::text || ')';
+ exception
+ when sqlstate 'U9999' then
+ raise notice 'SQLSTATE = U9999: tg_depth = %', tg_depth;
+ end;
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ execute 'insert into tg_depth_c values (' || new.id::text || ')';
+ return new;
+ end;
+ $$;
+ create trigger tg_depth_b_tr before insert on tg_depth_b
+ for each row execute procedure tg_depth_b_tf();
+
+ create function tg_depth_c_tf() returns trigger
+ language plpgsql as $$
+ begin
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ raise exception sqlstate 'U9999';
+ return new;
+ end;
+ $$;
+ create trigger tg_depth_c_tr before insert on tg_depth_c
+ for each row execute procedure tg_depth_c_tf();
+
+ insert into tg_depth_a values (999);
+
+ drop table tg_depth_a, tg_depth_b, tg_depth_c;
+ drop function tg_depth_a_tf();
+ drop function tg_depth_b_tf();
+ drop function tg_depth_c_tf();
On Jan29, 2011, at 00:15 , Kevin Grittner wrote:
The people who write my paychecks have insisted on me chunking out
some items which are part of our long-term plan besides the one I've
been focusing on lately. Most of it isn't of interest to anyone
outside Wisconsin Courts, but this piece might be; so I'm posting it
and putting onto the first 9.2 CF. We'll be using it for
development starting Monday and in production in two or three
months, so it should be pretty well tested by July. :-)
Here is review of the patch.
The patch didn't apply cleanly anymore due to changes to the plpgsql
regression test. Merging the changes was trivial though. Updated
patch attached.
* Regression Tests
Since I had to merge them anyway, I started with looking at the
regression tests. If I'm reading them correctly, the second
'raise' in tg_depth_a_tf() is never executed. tg_depth_b_tf() ends
with an insert into tg_depth_c, which unconditionally throws
U9999. Thus, tg_depth_a_tf() never gets further than the insert
into tg_depth_b.
This effectively means that the test fails to verify that TG_DEPTH
is correctly reset after a non-exceptional return from a nested
trigger invokation.
* Implementation
Now to the implementation. The trigger depth is incremented before
calling the trigger function in ExecCallTriggerFunc() and
decremented right afterwards, which seems fine - apart from the
fact that the decrement is skipped in case of an error. The patch
handles that by saving respectively restoring the value of pg_depth in
PushTransaction() respectively {Commit,Abort}SubTransaction().
While I can't come up with a failure case for that approach, it seems
rather fragile - who's to say that nested trigger invocations correspond
that tightly to sub-transaction?
At the very least this needs a comment explaining why this is safe,
but I believe the same effect could be achieved more cleanly by
a TRY/CATCH block in ExecCallTriggerFunc().
* Other in-core PLs
As it stands, the patch only export tg_depth to plpgsql functions,
not to functions written in one of the other in-core PLs. It'd be
good to change that, I believe - otherwise the other PLs become
second-class citizens in the long run.
best regards,
Florian Pflug
Attachments:
tg_depth.v2.patchapplication/octet-stream; name=tg_depth.v2.patchDownload
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index eea6ec5..5be8086 100644
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*************** RAISE unique_violation USING MESSAGE = '
*** 3383,3388 ****
--- 3383,3398 ----
</varlistentry>
<varlistentry>
+ <term><varname>TG_DEPTH</varname></term>
+ <listitem>
+ <para>
+ Data type <type>integer</type>; the current number of levels of
+ nesting within trigger execution.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><varname>TG_NARGS</varname></term>
<listitem>
<para>
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 2ca1c14..273fcae 100644
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** typedef struct TransactionStateData
*** 147,152 ****
--- 147,153 ----
Oid prevUser; /* previous CurrentUserId setting */
int prevSecContext; /* previous SecurityRestrictionContext */
bool prevXactReadOnly; /* entry-time xact r/o state */
+ int prevTgDepth; /* previous trigger depth */
bool startedInRecovery; /* did we start in recovery? */
struct TransactionStateData *parent; /* back link to parent */
} TransactionStateData;
*************** static TransactionStateData TopTransacti
*** 176,181 ****
--- 177,183 ----
InvalidOid, /* previous CurrentUserId setting */
0, /* previous SecurityRestrictionContext */
false, /* entry-time xact r/o state */
+ 0, /* previous trigger depth */
false, /* startedInRecovery */
NULL /* link to parent state block */
};
*************** CommitSubTransaction(void)
*** 4039,4044 ****
--- 4041,4048 ----
*/
XactReadOnly = s->prevXactReadOnly;
+ tg_depth = s->prevTgDepth;
+
CurrentResourceOwner = s->parent->curTransactionOwner;
CurTransactionResourceOwner = s->parent->curTransactionOwner;
ResourceOwnerDelete(s->curTransactionOwner);
*************** AbortSubTransaction(void)
*** 4157,4162 ****
--- 4161,4168 ----
*/
XactReadOnly = s->prevXactReadOnly;
+ tg_depth = s->prevTgDepth;
+
RESUME_INTERRUPTS();
}
*************** PushTransaction(void)
*** 4239,4244 ****
--- 4245,4251 ----
s->blockState = TBLOCK_SUBBEGIN;
GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext);
s->prevXactReadOnly = XactReadOnly;
+ s->prevTgDepth = tg_depth;
CurrentTransactionState = s;
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index ce36ea8..a9cb0a0 100644
*** a/src/backend/commands/trigger.c
--- b/src/backend/commands/trigger.c
***************
*** 56,61 ****
--- 56,64 ----
#include "utils/tqual.h"
+ /* How many levels deep into trigger execution are we? */
+ int tg_depth = 0;
+
/* GUC variables */
int SessionReplicationRole = SESSION_REPLICATION_ROLE_ORIGIN;
*************** ExecCallTriggerFunc(TriggerData *trigdat
*** 1811,1816 ****
--- 1814,1821 ----
if (instr)
InstrStartNode(instr + tgindx);
+ tg_depth++;
+
/*
* Do the function evaluation in the per-tuple memory context, so that
* leaked memory will be reclaimed once per tuple. Note in particular that
*************** ExecCallTriggerFunc(TriggerData *trigdat
*** 1833,1838 ****
--- 1838,1845 ----
MemoryContextSwitchTo(oldContext);
+ tg_depth--;
+
/*
* Trigger protocol allows function to return a null pointer, but NOT to
* set the isnull result flag.
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index b7649c6..a784b32 100644
*** a/src/backend/tcop/pquery.c
--- b/src/backend/tcop/pquery.c
*************** PortalRun(Portal portal, long count, boo
*** 738,743 ****
--- 738,745 ----
errmsg("portal \"%s\" cannot be run", portal->name)));
portal->status = PORTAL_ACTIVE;
+ tg_depth = 0;
+
/*
* Set up global portal context pointers.
*
*************** PortalRunFetch(Portal portal,
*** 1401,1406 ****
--- 1403,1410 ----
errmsg("portal \"%s\" cannot be run", portal->name)));
portal->status = PORTAL_ACTIVE;
+ tg_depth = 0;
+
/*
* Set up global portal context pointers.
*/
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index ad97871..349341e 100644
*** a/src/include/commands/trigger.h
--- b/src/include/commands/trigger.h
*************** extern PGDLLIMPORT int SessionReplicatio
*** 108,113 ****
--- 108,115 ----
#define TRIGGER_FIRES_ON_REPLICA 'R'
#define TRIGGER_DISABLED 'D'
+ extern int tg_depth;
+
extern Oid CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
Oid constraintOid, Oid indexOid,
bool isInternal);
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 75098ec..83c60e8 100644
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
*************** do_compile(FunctionCallInfo fcinfo,
*** 655,660 ****
--- 655,668 ----
true);
function->tg_table_schema_varno = var->dno;
+ /* add the variable tg_depth */
+ var = plpgsql_build_variable("tg_depth", 0,
+ plpgsql_build_datatype(INT4OID,
+ -1,
+ InvalidOid),
+ true);
+ function->tg_depth_varno = var->dno;
+
/* Add the variable tg_nargs */
var = plpgsql_build_variable("tg_nargs", 0,
plpgsql_build_datatype(INT4OID,
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 906a485..13f7238 100644
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
*************** plpgsql_exec_trigger(PLpgSQL_function *f
*** 627,632 ****
--- 627,637 ----
var->isnull = false;
var->freeval = true;
+ var = (PLpgSQL_var *) (estate.datums[func->tg_depth_varno]);
+ var->value = Int16GetDatum(tg_depth);
+ var->isnull = false;
+ var->freeval = false;
+
var = (PLpgSQL_var *) (estate.datums[func->tg_nargs_varno]);
var->value = Int16GetDatum(trigdata->tg_trigger->tgnargs);
var->isnull = false;
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index 89103ae..a5512fb 100644
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
*************** typedef struct PLpgSQL_function
*** 690,695 ****
--- 690,696 ----
int tg_relname_varno;
int tg_table_name_varno;
int tg_table_schema_varno;
+ int tg_depth_varno;
int tg_nargs_varno;
int tg_argv_varno;
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index bfabcbc..1f36944 100644
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
*************** NOTICE: {"(35,78)","(88,76)"}
*** 4434,4436 ****
--- 4434,4511 ----
drop function foreach_test(anyarray);
drop type xy_tuple;
+ -- Test TG_DEPTH
+ create table tg_depth_a (id int not null primary key);
+ NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "tg_depth_a_pkey" for table "tg_depth_a"
+ create table tg_depth_b (id int not null primary key);
+ NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "tg_depth_b_pkey" for table "tg_depth_b"
+ create table tg_depth_c (id int not null primary key);
+ NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "tg_depth_c_pkey" for table "tg_depth_c"
+ create function tg_depth_a_tf() returns trigger
+ language plpgsql as $$
+ begin
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ insert into tg_depth_b values (new.id);
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ return new;
+ end;
+ $$;
+ create trigger tg_depth_a_tr before insert on tg_depth_a
+ for each row execute procedure tg_depth_a_tf();
+ create function tg_depth_b_tf() returns trigger
+ language plpgsql as $$
+ begin
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ begin
+ execute 'insert into tg_depth_c values (' || new.id::text || ')';
+ exception
+ when sqlstate 'U9999' then
+ raise notice 'SQLSTATE = U9999: tg_depth = %', tg_depth;
+ end;
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ execute 'insert into tg_depth_c values (' || new.id::text || ')';
+ return new;
+ end;
+ $$;
+ create trigger tg_depth_b_tr before insert on tg_depth_b
+ for each row execute procedure tg_depth_b_tf();
+ create function tg_depth_c_tf() returns trigger
+ language plpgsql as $$
+ begin
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ raise exception sqlstate 'U9999';
+ return new;
+ end;
+ $$;
+ create trigger tg_depth_c_tr before insert on tg_depth_c
+ for each row execute procedure tg_depth_c_tf();
+ insert into tg_depth_a values (999);
+ NOTICE: tg_depth_a_tr: tg_depth = 1
+ NOTICE: tg_depth_b_tr: tg_depth = 2
+ CONTEXT: SQL statement "insert into tg_depth_b values (new.id)"
+ PL/pgSQL function "tg_depth_a_tf" line 4 at SQL statement
+ NOTICE: tg_depth_c_tr: tg_depth = 3
+ CONTEXT: SQL statement "insert into tg_depth_c values (999)"
+ PL/pgSQL function "tg_depth_b_tf" line 5 at EXECUTE statement
+ SQL statement "insert into tg_depth_b values (new.id)"
+ PL/pgSQL function "tg_depth_a_tf" line 4 at SQL statement
+ NOTICE: SQLSTATE = U9999: tg_depth = 2
+ CONTEXT: SQL statement "insert into tg_depth_b values (new.id)"
+ PL/pgSQL function "tg_depth_a_tf" line 4 at SQL statement
+ NOTICE: tg_depth_b_tr: tg_depth = 2
+ CONTEXT: SQL statement "insert into tg_depth_b values (new.id)"
+ PL/pgSQL function "tg_depth_a_tf" line 4 at SQL statement
+ NOTICE: tg_depth_c_tr: tg_depth = 3
+ CONTEXT: SQL statement "insert into tg_depth_c values (999)"
+ PL/pgSQL function "tg_depth_b_tf" line 11 at EXECUTE statement
+ SQL statement "insert into tg_depth_b values (new.id)"
+ PL/pgSQL function "tg_depth_a_tf" line 4 at SQL statement
+ ERROR: U9999
+ CONTEXT: SQL statement "insert into tg_depth_c values (999)"
+ PL/pgSQL function "tg_depth_b_tf" line 11 at EXECUTE statement
+ SQL statement "insert into tg_depth_b values (new.id)"
+ PL/pgSQL function "tg_depth_a_tf" line 4 at SQL statement
+ drop table tg_depth_a, tg_depth_b, tg_depth_c;
+ drop function tg_depth_a_tf();
+ drop function tg_depth_b_tf();
+ drop function tg_depth_c_tf();
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 14fb457..46ebef1 100644
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
*************** select foreach_test(ARRAY[[(10,20),(40,6
*** 3489,3491 ****
--- 3489,3547 ----
drop function foreach_test(anyarray);
drop type xy_tuple;
+
+
+ -- Test TG_DEPTH
+
+ create table tg_depth_a (id int not null primary key);
+ create table tg_depth_b (id int not null primary key);
+ create table tg_depth_c (id int not null primary key);
+
+ create function tg_depth_a_tf() returns trigger
+ language plpgsql as $$
+ begin
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ insert into tg_depth_b values (new.id);
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ return new;
+ end;
+ $$;
+ create trigger tg_depth_a_tr before insert on tg_depth_a
+ for each row execute procedure tg_depth_a_tf();
+
+ create function tg_depth_b_tf() returns trigger
+ language plpgsql as $$
+ begin
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ begin
+ execute 'insert into tg_depth_c values (' || new.id::text || ')';
+ exception
+ when sqlstate 'U9999' then
+ raise notice 'SQLSTATE = U9999: tg_depth = %', tg_depth;
+ end;
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ execute 'insert into tg_depth_c values (' || new.id::text || ')';
+ return new;
+ end;
+ $$;
+ create trigger tg_depth_b_tr before insert on tg_depth_b
+ for each row execute procedure tg_depth_b_tf();
+
+ create function tg_depth_c_tf() returns trigger
+ language plpgsql as $$
+ begin
+ raise notice '%: tg_depth = %', tg_name, tg_depth;
+ raise exception sqlstate 'U9999';
+ return new;
+ end;
+ $$;
+ create trigger tg_depth_c_tr before insert on tg_depth_c
+ for each row execute procedure tg_depth_c_tf();
+
+ insert into tg_depth_a values (999);
+
+ drop table tg_depth_a, tg_depth_b, tg_depth_c;
+ drop function tg_depth_a_tf();
+ drop function tg_depth_b_tf();
+ drop function tg_depth_c_tf();
+
Florian Pflug <fgp@phlo.org> wrote:
Here is review of the patch.
Thanks for the review. I think I'd better try to keep the decks
clear for dealing with any SSI issues which may surface during the
coming month, so I'm going to mark this patch "Returned with
Feedback" and look at this for possible resubmission in light of
your review in a later CF.
On resubmit I will start with a reference to your review and proceed
with discussion from there.
Thanks,
-Kevin
[reviving discussion of another old patch]
In post:
http://archives.postgresql.org/pgsql-hackers/2011-06/msg00870.php
Florian Pflug <fgp@phlo.org> wrote:
Updated patch attached.
Thanks.
The trigger depth is incremented before calling the trigger
function in ExecCallTriggerFunc() and decremented right
afterwards, which seems fine - apart from the fact that the
decrement is skipped in case of an error. The patch handles that
by saving respectively restoring the value of pg_depth in
PushTransaction() respectively {Commit,Abort}SubTransaction().While I can't come up with a failure case for that approach, it
seems rather fragile - who's to say that nested trigger
invocations correspond that tightly to sub-transaction?
Good question -- is it reasonable to assume that if an error is
thrown in a trigger, that the state restored when the error is
caught will be at the same trigger depth as when the transaction or
subtransaction started?
FWIW, the patch, using this technique, has been in production use
with about 3,000 OLTP users for about six months, with no apparent
problems. On the other hand, we don't do a lot with explicit
subtransactions.
At the very least this needs a comment explaining why this is
safe,
Agreed.
but I believe the same effect could be achieved more cleanly by
a TRY/CATCH block in ExecCallTriggerFunc().
That occurred to me, but I was concerned about the cost of setting
and clearing a longjump target for every trigger call. It seems
like I've seen comments about that being relatively expensive.
Tracking one more int in the current subtransaction structures is
pretty cheap, if that works.
* Other in-core PLs
As it stands, the patch only export tg_depth to plpgsql functions,
not to functions written in one of the other in-core PLs. It'd be
good to change that, I believe - otherwise the other PLs become
second-class citizens in the long run.
Are you suggesting that this be implemented as a special trigger
variable in every PL, or that it simply be a regular function that
returns zero when not in a trigger and some positive value when
called from a trigger? The latter seems like a pretty good idea to
me. If that is done, is there any point to *also* having a TG_DEPTH
trigger variable in plpgsql? (I don't think there is.)
-Kevin