Hi Rajkumar,
Thanks for testing and the report.
On 2017/04/21 17:00, Rajkumar Raghuwanshi wrote:
> Hi,
>
> I have observed below with the statement triggers.
>
> I am able to create statement triggers at root partition, but these
> triggers, not getting fired on updating partition.
>
> CREATE TABLE pt (a INT, b INT) PARTITION BY RANGE(a);
> CREATE TABLE pt1 PARTITION OF pt FOR VALUES FROM (1) to (7);
> CREATE TABLE pt2 PARTITION OF pt FOR VALUES FROM (7) to (11);
> INSERT INTO pt (a,b) SELECT i,i FROM generate_series(1,10)i;
> CREATE TABLE pt_trigger(TG_NAME varchar,TG_TABLE_NAME varchar,TG_LEVEL
> varchar,TG_WHEN varchar);
> CREATE OR REPLACE FUNCTION process_pt_trigger() RETURNS TRIGGER AS $pttg$
> BEGIN
> IF (TG_OP = 'UPDATE') THEN
> INSERT INTO pt_trigger SELECT
> TG_NAME,TG_TABLE_NAME,TG_LEVEL,TG_WHEN;
> RETURN NEW;
> END IF;
> RETURN NULL;
> END;
> $pttg$ LANGUAGE plpgsql;
> CREATE TRIGGER pt_trigger_after_p0 AFTER UPDATE ON pt FOR EACH STATEMENT
> EXECUTE PROCEDURE process_pt_trigger();
> CREATE TRIGGER pt_trigger_before_p0 BEFORE UPDATE ON pt FOR EACH STATEMENT
> EXECUTE PROCEDURE process_pt_trigger();
>
> postgres=# UPDATE pt SET a = 2 WHERE a = 1;
> UPDATE 1
> postgres=# SELECT * FROM pt_trigger ORDER BY 1;
> tg_name | tg_table_name | tg_level | tg_when
> ---------+---------------+----------+---------
> (0 rows)
>
> no statement level trigger fired in this case, is this expected behaviour??
I think we discussed this during the development and decided to allow
per-statement triggers on partitioned tables [1], but it seems it doesn't
quite work for update/delete as your example shows. You can however see
that per-statement triggers of the root partitioned table *are* fired in
the case of insert.
The reason it doesn't work is that we do not allocate ResultRelInfos for
partitioned tables (not even for the root partitioned table in the
update/delete cases), because the current implementation assumes they are
not required. That's fine only so long as we consider that no rows are
inserted into them, no indexes need to be updated, and that no row-level
triggers are to be fired. But it misses the fact that we do allow
statement-level triggers on partitioned tables. So, we should fix things
such that ResultRelInfos are allocated so that any statement-level
triggers are fired. But there are following questions to consider:
1. Should we consider only the root partitioned table or all partitioned
tables in a given hierarchy, including the child partitioned tables?
Note that this is related to a current limitation of updating/deleting
inheritance hierarchies that we do not currently fire per-statement
triggers of the child tables. See the TODO item in wiki:
https://wiki.postgresql.org/wiki/Todo#Triggers, which says: "When
statement-level triggers are defined on a parent table, have them fire
only on the parent table, and fire child table triggers only where
appropriate"
2. Do we apply the same to inserts on the partitioned tables? Since
insert on a partitioned table implicitly affects its partitions, one
might say that we would need to fire per-statement insert triggers of
all the partitions.
Meanwhile, attached is a set of patches to fix this. Descriptions follow:
0001: fire per-statement triggers of the partitioned table mentioned in a
given update or delete statement
0002: fire per-statement triggers of the child tables during update/delete
of inheritance hierarchies (including the partitioned table case)
Depending on the answer to 2 above, we can arrange for the per-statement
triggers of all the partitions to be fired upon insert into the
partitioned table.
> but if i am creating triggers on leaf partition, trigger is getting fired.
>
> CREATE TRIGGER pt_trigger_after_p1 AFTER UPDATE ON pt1 FOR EACH STATEMENT
> EXECUTE PROCEDURE process_pt_trigger();
> CREATE TRIGGER pt_trigger_before_p1 BEFORE UPDATE ON pt1 FOR EACH STATEMENT
> EXECUTE PROCEDURE process_pt_trigger();
>
> postgres=# UPDATE pt SET a = 5 WHERE a = 4;
> UPDATE 1
> postgres=# SELECT * FROM pt_trigger ORDER BY 1;
> tg_name | tg_table_name | tg_level | tg_when
> ----------------------+---------------+-----------+---------
> pt_trigger_after_p1 | pt1 | STATEMENT | AFTER
> pt_trigger_before_p1 | pt1 | STATEMENT | BEFORE
> (2 rows)
Actually, this works only by accident (with the current implementation,
the *first* partition's triggers will get fired). If you create another
partition, its per-statement triggers won't get fired. The patches
described above fixes that.
It would be great if you could check if the patches fix the issues.
Also, adding this to the PostgreSQL 10 open items list.
Thanks,
Amit
[1]
https://www.postgresql.org/message-id/8e05817e-14c8-13e4-502b-e440adb24258%40lab.ntt.co.jp
>From 9c7543615ccb05c004591a9176f818413df7ea9d Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Mon, 24 Apr 2017 14:55:08 +0900
Subject: [PATCH 1/2] Fire per-statement triggers of partitioned tables
Current implementation completely misses them, because it assumes
that no ResultRelInfos need to be created for partitioned tables,
whereas they *are* needed to fire the per-statment triggers, which
we *do* allow to be defined on the partitioned tables.
Reported By: Rajkumar Raghuwanshi
Patch by: Amit Langote
Report: https://www.postgresql.org/message-id/CAKcux6%3DwYospCRY2J4XEFuVy0L41S%3Dfic7rmkbsU-GXhhSbmBg%40mail.gmail.com
---
src/backend/executor/execMain.c | 33 ++++++++++++++++-
src/backend/executor/nodeModifyTable.c | 66 ++++++++++++++++++++++++++++-----
src/backend/nodes/copyfuncs.c | 1 +
src/backend/nodes/outfuncs.c | 1 +
src/backend/nodes/readfuncs.c | 1 +
src/backend/optimizer/plan/createplan.c | 1 +
src/backend/optimizer/plan/setrefs.c | 4 ++
src/include/nodes/execnodes.h | 11 ++++++
src/include/nodes/plannodes.h | 2 +
src/test/regress/expected/triggers.out | 54 +++++++++++++++++++++++++++
src/test/regress/sql/triggers.sql | 48 ++++++++++++++++++++++++
11 files changed, 210 insertions(+), 12 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 5c12fb457d..586b396b3e 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -861,18 +861,35 @@ InitPlan(QueryDesc *queryDesc, int eflags)
/*
* In the partitioned result relation case, lock the non-leaf result
- * relations too. We don't however need ResultRelInfos for them.
+ * relations too. We also need ResultRelInfos so that per-statement
+ * triggers defined on these relations can be fired.
*/
if (plannedstmt->nonleafResultRelations)
{
+ numResultRelations =
+ list_length(plannedstmt->nonleafResultRelations);
+
+ resultRelInfos = (ResultRelInfo *)
+ palloc(numResultRelations * sizeof(ResultRelInfo));
+ resultRelInfo = resultRelInfos;
foreach(l, plannedstmt->nonleafResultRelations)
{
Index resultRelationIndex = lfirst_int(l);
Oid resultRelationOid;
+ Relation resultRelation;
resultRelationOid = getrelid(resultRelationIndex, rangeTable);
- LockRelationOid(resultRelationOid, RowExclusiveLock);
+ resultRelation = heap_open(resultRelationOid, RowExclusiveLock);
+ InitResultRelInfo(resultRelInfo,
+ resultRelation,
+ resultRelationIndex,
+ NULL,
+ estate->es_instrument);
+ resultRelInfo++;
}
+
+ estate->es_nonleaf_result_relations = resultRelInfos;
+ estate->es_num_nonleaf_result_relations = numResultRelations;
}
}
else
@@ -883,6 +900,8 @@ InitPlan(QueryDesc *queryDesc, int eflags)
estate->es_result_relations = NULL;
estate->es_num_result_relations = 0;
estate->es_result_relation_info = NULL;
+ estate->es_nonleaf_result_relations = NULL;
+ estate->es_num_nonleaf_result_relations = 0;
}
/*
@@ -1566,6 +1585,16 @@ ExecEndPlan(PlanState *planstate, EState *estate)
}
/*
+ * close any non-leaf target relations
+ */
+ resultRelInfo = estate->es_nonleaf_result_relations;
+ for (i = estate->es_num_nonleaf_result_relations; i > 0; i--)
+ {
+ heap_close(resultRelInfo->ri_RelationDesc, NoLock);
+ resultRelInfo++;
+ }
+
+ /*
* likewise close any trigger target relations
*/
foreach(l, estate->es_trig_target_relations)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 71e3b8ec2d..6cab15646f 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -62,6 +62,8 @@ static bool ExecOnConflictUpdate(ModifyTableState *mtstate,
EState *estate,
bool canSetTag,
TupleTableSlot **returning);
+static void fireBSTriggersFor(ModifyTableState *node, ResultRelInfo *resultRelInfo);
+static void fireASTriggersFor(ModifyTableState *node, ResultRelInfo *resultRelInfo);
/*
* Verify that the tuples to be produced by INSERT or UPDATE match the
@@ -1328,19 +1330,39 @@ ExecOnConflictUpdate(ModifyTableState *mtstate,
static void
fireBSTriggers(ModifyTableState *node)
{
+ /* Fire for the root partitioned table, if any, and return. */
+ if (node->nonleafResultRelInfo)
+ {
+ fireBSTriggersFor(node, node->nonleafResultRelInfo);
+ return;
+ }
+
+ /*
+ * Fire for the main result relation. In the partitioned table case,
+ * the following happens to be the first leaf partition, which we don't
+ * need to fire triggers for.
+ */
+ fireBSTriggersFor(node, node->resultRelInfo);
+}
+
+/*
+ * Process BEFORE EACH STATEMENT triggers for one result relation
+ */
+static void
+fireBSTriggersFor(ModifyTableState *node, ResultRelInfo *resultRelInfo)
+{
switch (node->operation)
{
case CMD_INSERT:
- ExecBSInsertTriggers(node->ps.state, node->resultRelInfo);
+ ExecBSInsertTriggers(node->ps.state, resultRelInfo);
if (node->mt_onconflict == ONCONFLICT_UPDATE)
- ExecBSUpdateTriggers(node->ps.state,
- node->resultRelInfo);
+ ExecBSUpdateTriggers(node->ps.state, resultRelInfo);
break;
case CMD_UPDATE:
- ExecBSUpdateTriggers(node->ps.state, node->resultRelInfo);
+ ExecBSUpdateTriggers(node->ps.state, resultRelInfo);
break;
case CMD_DELETE:
- ExecBSDeleteTriggers(node->ps.state, node->resultRelInfo);
+ ExecBSDeleteTriggers(node->ps.state, resultRelInfo);
break;
default:
elog(ERROR, "unknown operation");
@@ -1354,19 +1376,39 @@ fireBSTriggers(ModifyTableState *node)
static void
fireASTriggers(ModifyTableState *node)
{
+ /* Fire for the root partitioned table, if any, and return. */
+ if (node->nonleafResultRelInfo)
+ {
+ fireASTriggersFor(node, node->nonleafResultRelInfo);
+ return;
+ }
+
+ /*
+ * Fire for the main result relation. In the partitioned table case,
+ * the following happens to be the first leaf partition, which we don't
+ * need to fire triggers for.
+ */
+ fireASTriggersFor(node, node->resultRelInfo);
+}
+
+/*
+ * Process AFTER EACH STATEMENT triggers for one result relation
+ */
+static void
+fireASTriggersFor(ModifyTableState *node, ResultRelInfo *resultRelInfo)
+{
switch (node->operation)
{
case CMD_INSERT:
if (node->mt_onconflict == ONCONFLICT_UPDATE)
- ExecASUpdateTriggers(node->ps.state,
- node->resultRelInfo);
- ExecASInsertTriggers(node->ps.state, node->resultRelInfo);
+ ExecASUpdateTriggers(node->ps.state, resultRelInfo);
+ ExecASInsertTriggers(node->ps.state, resultRelInfo);
break;
case CMD_UPDATE:
- ExecASUpdateTriggers(node->ps.state, node->resultRelInfo);
+ ExecASUpdateTriggers(node->ps.state, resultRelInfo);
break;
case CMD_DELETE:
- ExecASDeleteTriggers(node->ps.state, node->resultRelInfo);
+ ExecASDeleteTriggers(node->ps.state, resultRelInfo);
break;
default:
elog(ERROR, "unknown operation");
@@ -1628,6 +1670,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
ModifyTableState *mtstate;
CmdType operation = node->operation;
int nplans = list_length(node->plans);
+ int num_nonleafrels = list_length(node->partitioned_rels);
ResultRelInfo *saved_resultRelInfo;
ResultRelInfo *resultRelInfo;
TupleDesc tupDesc;
@@ -1652,6 +1695,9 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
mtstate->mt_plans = (PlanState **) palloc0(sizeof(PlanState *) * nplans);
mtstate->resultRelInfo = estate->es_result_relations + node->resultRelIndex;
+ mtstate->mt_numnonleafrels = num_nonleafrels;
+ mtstate->nonleafResultRelInfo = estate->es_nonleaf_result_relations +
+ node->nonleafResultRelIndex;
mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans);
mtstate->mt_nplans = nplans;
mtstate->mt_onconflict = node->onConflictAction;
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 00a0fed23d..e425a57feb 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -205,6 +205,7 @@ _copyModifyTable(const ModifyTable *from)
COPY_NODE_FIELD(partitioned_rels);
COPY_NODE_FIELD(resultRelations);
COPY_SCALAR_FIELD(resultRelIndex);
+ COPY_SCALAR_FIELD(nonleafResultRelIndex);
COPY_NODE_FIELD(plans);
COPY_NODE_FIELD(withCheckOptionLists);
COPY_NODE_FIELD(returningLists);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 28cef85579..a501f250ba 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -350,6 +350,7 @@ _outModifyTable(StringInfo str, const ModifyTable *node)
WRITE_NODE_FIELD(partitioned_rels);
WRITE_NODE_FIELD(resultRelations);
WRITE_INT_FIELD(resultRelIndex);
+ WRITE_INT_FIELD(nonleafResultRelIndex);
WRITE_NODE_FIELD(plans);
WRITE_NODE_FIELD(withCheckOptionLists);
WRITE_NODE_FIELD(returningLists);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index a883220a49..aa6f54870c 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -1548,6 +1548,7 @@ _readModifyTable(void)
READ_NODE_FIELD(partitioned_rels);
READ_NODE_FIELD(resultRelations);
READ_INT_FIELD(resultRelIndex);
+ READ_INT_FIELD(nonleafResultRelIndex);
READ_NODE_FIELD(plans);
READ_NODE_FIELD(withCheckOptionLists);
READ_NODE_FIELD(returningLists);
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 95e6eb7d28..de2c77a22c 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -6437,6 +6437,7 @@ make_modifytable(PlannerInfo *root,
node->partitioned_rels = partitioned_rels;
node->resultRelations = resultRelations;
node->resultRelIndex = -1; /* will be set correctly in setrefs.c */
+ node->nonleafResultRelIndex = -1; /* will be set correctly in setrefs.c */
node->plans = subplans;
if (!onconflict)
{
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 1278371b65..27a145187e 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -883,7 +883,11 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
* If the main target relation is a partitioned table, the
* following list contains the RT indexes of partitioned child
* relations, which are not included in the above list.
+ * Set nonleafResultRelIndex to reflect the starting position
+ * in the global list.
*/
+ splan->nonleafResultRelIndex =
+ list_length(root->glob->nonleafResultRelations);
root->glob->nonleafResultRelations =
list_concat(root->glob->nonleafResultRelations,
list_copy(splan->partitioned_rels));
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 4330a851c3..3f801b9b0c 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -422,6 +422,14 @@ typedef struct EState
int es_num_result_relations; /* length of array */
ResultRelInfo *es_result_relation_info; /* currently active array elt */
+ /*
+ * Info about non-leaf target tables during update/delete on partitioned
+ * tables. They are required only to fire the per-statement triggers
+ * defined on the non-leaf tables in a partitioned table hierarchy.
+ */
+ ResultRelInfo *es_nonleaf_result_relations; /* array of ResultRelInfos */
+ int es_num_nonleaf_result_relations; /* length of array */
+
/* Stuff used for firing triggers: */
List *es_trig_target_relations; /* trigger-only ResultRelInfos */
TupleTableSlot *es_trig_tuple_slot; /* for trigger output tuples */
@@ -914,6 +922,9 @@ typedef struct ModifyTableState
int mt_nplans; /* number of plans in the array */
int mt_whichplan; /* which one is being executed (0..n-1) */
ResultRelInfo *resultRelInfo; /* per-subplan target relations */
+ int mt_numnonleafrels; /* number of non-leaf result rels in the
+ * below array */
+ ResultRelInfo *nonleafResultRelInfo; /* non-leaf target relations */
List **mt_arowmarks; /* per-subplan ExecAuxRowMark lists */
EPQState mt_epqstate; /* for evaluating EvalPlanQual rechecks */
bool fireBSTriggers; /* do we need to fire stmt triggers? */
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index cba915572e..474343d340 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -211,6 +211,8 @@ typedef struct ModifyTable
List *partitioned_rels;
List *resultRelations; /* integer list of RT indexes */
int resultRelIndex; /* index of first resultRel in plan's list */
+ int nonleafResultRelIndex; /* index of first nonleaf resultRel in
+ * plan's list */
List *plans; /* plan(s) producing source data */
List *withCheckOptionLists; /* per-target-table WCO lists */
List *returningLists; /* per-target-table RETURNING tlists */
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 4b0b3b7c42..ec0e0c2782 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -1787,3 +1787,57 @@ create trigger my_trigger after update on my_table_42 referencing old table as o
drop trigger my_trigger on my_table_42;
drop table my_table_42;
drop table my_table;
+--
+-- Verify that per-statement triggers are fired for partitioned tables
+--
+create table parted_stmt_trig (a int) partition by list (a);
+create table parted_stmt_trig1 partition of parted_stmt_trig for values in (1);
+create table parted_stmt_trig2 partition of parted_stmt_trig for values in (2);
+create or replace function trigger_notice() returns trigger as $$
+ begin raise notice 'trigger on % % %', TG_TABLE_NAME, TG_WHEN, TG_OP; return null; end;
+ $$ language plpgsql;
+-- insert/update/delete triggers on the parent
+create trigger trig_ins_before before insert on parted_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_ins_after after insert on parted_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_upd_before before update on parted_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_upd_after after update on parted_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_before before delete on parted_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_after after delete on parted_stmt_trig
+ for each statement execute procedure trigger_notice();
+-- insert/update/delete triggers on the first partition
+create trigger trig_ins_before before insert on parted_stmt_trig1
+ for each statement execute procedure trigger_notice();
+create trigger trig_ins_after after insert on parted_stmt_trig1
+ for each statement execute procedure trigger_notice();
+create trigger trig_upd_before before update on parted_stmt_trig1
+ for each statement execute procedure trigger_notice();
+create trigger trig_upd_after after update on parted_stmt_trig1
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_before before delete on parted_stmt_trig1
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_after after delete on parted_stmt_trig1
+ for each statement execute procedure trigger_notice();
+with ins (partname, a) as (
+ insert into parted_stmt_trig values (1) returning tableoid::regclass, a
+) insert into parted_stmt_trig select a+1 from ins returning tableoid::regclass, a;
+NOTICE: trigger on parted_stmt_trig BEFORE INSERT
+NOTICE: trigger on parted_stmt_trig BEFORE INSERT
+NOTICE: trigger on parted_stmt_trig AFTER INSERT
+NOTICE: trigger on parted_stmt_trig AFTER INSERT
+ tableoid | a
+-------------------+---
+ parted_stmt_trig2 | 2
+(1 row)
+
+update parted_stmt_trig set a = a;
+NOTICE: trigger on parted_stmt_trig BEFORE UPDATE
+NOTICE: trigger on parted_stmt_trig AFTER UPDATE
+delete from parted_stmt_trig;
+NOTICE: trigger on parted_stmt_trig BEFORE DELETE
+NOTICE: trigger on parted_stmt_trig AFTER DELETE
+drop table parted_stmt_trig;
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 4473ce0518..a767098465 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1263,3 +1263,51 @@ create trigger my_trigger after update on my_table_42 referencing old table as o
drop trigger my_trigger on my_table_42;
drop table my_table_42;
drop table my_table;
+
+--
+-- Verify that per-statement triggers are fired for partitioned tables
+--
+create table parted_stmt_trig (a int) partition by list (a);
+create table parted_stmt_trig1 partition of parted_stmt_trig for values in (1);
+create table parted_stmt_trig2 partition of parted_stmt_trig for values in (2);
+
+create or replace function trigger_notice() returns trigger as $$
+ begin raise notice 'trigger on % % %', TG_TABLE_NAME, TG_WHEN, TG_OP; return null; end;
+ $$ language plpgsql;
+
+-- insert/update/delete triggers on the parent
+create trigger trig_ins_before before insert on parted_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_ins_after after insert on parted_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_upd_before before update on parted_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_upd_after after update on parted_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_before before delete on parted_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_after after delete on parted_stmt_trig
+ for each statement execute procedure trigger_notice();
+
+-- insert/update/delete triggers on the first partition
+create trigger trig_ins_before before insert on parted_stmt_trig1
+ for each statement execute procedure trigger_notice();
+create trigger trig_ins_after after insert on parted_stmt_trig1
+ for each statement execute procedure trigger_notice();
+create trigger trig_upd_before before update on parted_stmt_trig1
+ for each statement execute procedure trigger_notice();
+create trigger trig_upd_after after update on parted_stmt_trig1
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_before before delete on parted_stmt_trig1
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_after after delete on parted_stmt_trig1
+ for each statement execute procedure trigger_notice();
+
+with ins (partname, a) as (
+ insert into parted_stmt_trig values (1) returning tableoid::regclass, a
+) insert into parted_stmt_trig select a+1 from ins returning tableoid::regclass, a;
+
+update parted_stmt_trig set a = a;
+delete from parted_stmt_trig;
+
+drop table parted_stmt_trig;
--
2.11.0
>From f69acdb660e391302caf9cfe4086a4ae7f4122b2 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Mon, 24 Apr 2017 13:57:50 +0900
Subject: [PATCH 2/2] Fire per-statement triggers of inheritance child tables
There has been a long-standing decision to not fire the child table's
per-statement triggers even though update/delete on inheritance
hierarchies (including the partitioning case) *do* affect the child
tables along with the parent table mentioned in the statement.
---
src/backend/executor/nodeModifyTable.c | 32 ++++++++++++++++-------------
src/test/regress/expected/triggers.out | 37 ++++++++++++++++++++++++++++++++++
src/test/regress/sql/triggers.sql | 29 ++++++++++++++++++++++++++
3 files changed, 84 insertions(+), 14 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 6cab15646f..cbaf402e62 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1330,19 +1330,21 @@ ExecOnConflictUpdate(ModifyTableState *mtstate,
static void
fireBSTriggers(ModifyTableState *node)
{
- /* Fire for the root partitioned table, if any, and return. */
+ int i;
+
+ /* Fire for the non-leaf targets, including the root partitioned table */
if (node->nonleafResultRelInfo)
{
- fireBSTriggersFor(node, node->nonleafResultRelInfo);
- return;
+ for (i = 0; i < node->mt_numnonleafrels; i++)
+ fireBSTriggersFor(node, node->nonleafResultRelInfo + i);
}
/*
- * Fire for the main result relation. In the partitioned table case,
- * the following happens to be the first leaf partition, which we don't
- * need to fire triggers for.
+ * Fire for the leaf targets. In the non-partitioned inheritance case,
+ * this includes all the tables in the hierarchy.
*/
- fireBSTriggersFor(node, node->resultRelInfo);
+ for (i = 0; i < node->mt_nplans; i++)
+ fireBSTriggersFor(node, node->resultRelInfo + i);
}
/*
@@ -1376,19 +1378,21 @@ fireBSTriggersFor(ModifyTableState *node, ResultRelInfo *resultRelInfo)
static void
fireASTriggers(ModifyTableState *node)
{
- /* Fire for the root partitioned table, if any, and return. */
+ int i;
+
+ /* Fire for the non-leaf targets, including the root partitioned table */
if (node->nonleafResultRelInfo)
{
- fireASTriggersFor(node, node->nonleafResultRelInfo);
- return;
+ for (i = 0; i < node->mt_numnonleafrels; i++)
+ fireASTriggersFor(node, node->nonleafResultRelInfo + i);
}
/*
- * Fire for the main result relation. In the partitioned table case,
- * the following happens to be the first leaf partition, which we don't
- * need to fire triggers for.
+ * Fire for the leaf targets. In the non-partitioned inheritance case,
+ * this includes all the tables in the hierarchy.
*/
- fireASTriggersFor(node, node->resultRelInfo);
+ for (i = 0; i < node->mt_nplans; i++)
+ fireASTriggersFor(node, node->resultRelInfo + i);
}
/*
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index ec0e0c2782..df2726c497 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -1793,6 +1793,8 @@ drop table my_table;
create table parted_stmt_trig (a int) partition by list (a);
create table parted_stmt_trig1 partition of parted_stmt_trig for values in (1);
create table parted_stmt_trig2 partition of parted_stmt_trig for values in (2);
+create table regular_stmt_trig (a int);
+create table regular_stmt_trig_child () inherits (regular_stmt_trig);
create or replace function trigger_notice() returns trigger as $$
begin raise notice 'trigger on % % %', TG_TABLE_NAME, TG_WHEN, TG_OP; return null; end;
$$ language plpgsql;
@@ -1822,6 +1824,24 @@ create trigger trig_del_before before delete on parted_stmt_trig1
for each statement execute procedure trigger_notice();
create trigger trig_del_after after delete on parted_stmt_trig1
for each statement execute procedure trigger_notice();
+-- update/delete triggers on the regular inheritance parent
+create trigger trig_upd_before before update on regular_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_upd_after after update on regular_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_before before delete on regular_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_after after delete on regular_stmt_trig
+ for each statement execute procedure trigger_notice();
+-- update/delete triggers on the regular inheritance child
+create trigger trig_upd_before before update on regular_stmt_trig_child
+ for each statement execute procedure trigger_notice();
+create trigger trig_upd_after after update on regular_stmt_trig_child
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_before before delete on regular_stmt_trig_child
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_after after delete on regular_stmt_trig_child
+ for each statement execute procedure trigger_notice();
with ins (partname, a) as (
insert into parted_stmt_trig values (1) returning tableoid::regclass, a
) insert into parted_stmt_trig select a+1 from ins returning tableoid::regclass, a;
@@ -1836,8 +1856,25 @@ NOTICE: trigger on parted_stmt_trig AFTER INSERT
update parted_stmt_trig set a = a;
NOTICE: trigger on parted_stmt_trig BEFORE UPDATE
+NOTICE: trigger on parted_stmt_trig1 BEFORE UPDATE
NOTICE: trigger on parted_stmt_trig AFTER UPDATE
+NOTICE: trigger on parted_stmt_trig1 AFTER UPDATE
delete from parted_stmt_trig;
NOTICE: trigger on parted_stmt_trig BEFORE DELETE
+NOTICE: trigger on parted_stmt_trig1 BEFORE DELETE
NOTICE: trigger on parted_stmt_trig AFTER DELETE
+NOTICE: trigger on parted_stmt_trig1 AFTER DELETE
+insert into regular_stmt_trig values (1);
+insert into regular_stmt_trig_child values (1);
+update regular_stmt_trig set a = a;
+NOTICE: trigger on regular_stmt_trig BEFORE UPDATE
+NOTICE: trigger on regular_stmt_trig_child BEFORE UPDATE
+NOTICE: trigger on regular_stmt_trig AFTER UPDATE
+NOTICE: trigger on regular_stmt_trig_child AFTER UPDATE
+delete from regular_stmt_trig;
+NOTICE: trigger on regular_stmt_trig BEFORE DELETE
+NOTICE: trigger on regular_stmt_trig_child BEFORE DELETE
+NOTICE: trigger on regular_stmt_trig AFTER DELETE
+NOTICE: trigger on regular_stmt_trig_child AFTER DELETE
drop table parted_stmt_trig;
+drop table regular_stmt_trig, regular_stmt_trig_child;
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index a767098465..2784fe1077 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1271,6 +1271,9 @@ create table parted_stmt_trig (a int) partition by list (a);
create table parted_stmt_trig1 partition of parted_stmt_trig for values in (1);
create table parted_stmt_trig2 partition of parted_stmt_trig for values in (2);
+create table regular_stmt_trig (a int);
+create table regular_stmt_trig_child () inherits (regular_stmt_trig);
+
create or replace function trigger_notice() returns trigger as $$
begin raise notice 'trigger on % % %', TG_TABLE_NAME, TG_WHEN, TG_OP; return null; end;
$$ language plpgsql;
@@ -1303,6 +1306,26 @@ create trigger trig_del_before before delete on parted_stmt_trig1
create trigger trig_del_after after delete on parted_stmt_trig1
for each statement execute procedure trigger_notice();
+-- update/delete triggers on the regular inheritance parent
+create trigger trig_upd_before before update on regular_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_upd_after after update on regular_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_before before delete on regular_stmt_trig
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_after after delete on regular_stmt_trig
+ for each statement execute procedure trigger_notice();
+
+-- update/delete triggers on the regular inheritance child
+create trigger trig_upd_before before update on regular_stmt_trig_child
+ for each statement execute procedure trigger_notice();
+create trigger trig_upd_after after update on regular_stmt_trig_child
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_before before delete on regular_stmt_trig_child
+ for each statement execute procedure trigger_notice();
+create trigger trig_del_after after delete on regular_stmt_trig_child
+ for each statement execute procedure trigger_notice();
+
with ins (partname, a) as (
insert into parted_stmt_trig values (1) returning tableoid::regclass, a
) insert into parted_stmt_trig select a+1 from ins returning tableoid::regclass, a;
@@ -1310,4 +1333,10 @@ with ins (partname, a) as (
update parted_stmt_trig set a = a;
delete from parted_stmt_trig;
+insert into regular_stmt_trig values (1);
+insert into regular_stmt_trig_child values (1);
+update regular_stmt_trig set a = a;
+delete from regular_stmt_trig;
+
drop table parted_stmt_trig;
+drop table regular_stmt_trig, regular_stmt_trig_child;
--
2.11.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
