pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: allow trigger to get updated columns
Date: Mon, 24 Feb 2020 10:58:03 +0100
Message-ID: <11c5f156-67a9-0fb5-8200-2a8018eb2e0c@2ndquadrant.com> (raw )
This is a change to make the bitmap of updated columns available to a
trigger in TriggerData. This is the same idea as was recently done to
generated columns [0]: Generic triggers such as tsvector_update_trigger
can use this information to skip work if the columns they are interested
in haven't changed. With the generated columns change, perhaps this
isn't so interesting anymore, but I suspect a lot of existing
installations still use tsvector_update_trigger. In any case, since I
had already written the code, I figured I post it here. Perhaps there
are other use cases.
[0]:
https://www.postgresql.org/message-id/flat/b05e781a-fa16-6b52-6738-761181204567@2ndquadrant.com
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments: download all (2 files, .tar.gz)
0001-Code-simplification.patch (text/plain, 9.5K) download
From 0c901266a1b40a257320166fdacaaefd00e4dbce Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 24 Feb 2020 10:12:10 +0100
Subject: [PATCH 1/2] Code simplification
Initialize TriggerData to 0 for the whole struct together, instead of
each field separately.
---
src/backend/commands/tablecmds.c | 4 +-
src/backend/commands/trigger.c | 78 +++++---------------------------
2 files changed, 12 insertions(+), 70 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b7c8d663fc..6af984ff10 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10264,7 +10264,7 @@ validateForeignKeyConstraint(char *conname,
while (table_scan_getnextslot(scan, ForwardScanDirection, slot))
{
LOCAL_FCINFO(fcinfo, 0);
- TriggerData trigdata;
+ TriggerData trigdata = {0};
CHECK_FOR_INTERRUPTS();
@@ -10283,8 +10283,6 @@ validateForeignKeyConstraint(char *conname,
trigdata.tg_relation = rel;
trigdata.tg_trigtuple = ExecFetchSlotHeapTuple(slot, false, NULL);
trigdata.tg_trigslot = slot;
- trigdata.tg_newtuple = NULL;
- trigdata.tg_newslot = NULL;
trigdata.tg_trigger = &trig;
fcinfo->context = (Node *) &trigdata;
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index b9b1262e30..26593576fd 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2458,7 +2458,7 @@ ExecBSInsertTriggers(EState *estate, ResultRelInfo *relinfo)
{
TriggerDesc *trigdesc;
int i;
- TriggerData LocTriggerData;
+ TriggerData LocTriggerData = {0};
trigdesc = relinfo->ri_TrigDesc;
@@ -2476,12 +2476,6 @@ ExecBSInsertTriggers(EState *estate, ResultRelInfo *relinfo)
LocTriggerData.tg_event = TRIGGER_EVENT_INSERT |
TRIGGER_EVENT_BEFORE;
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
- LocTriggerData.tg_trigtuple = NULL;
- LocTriggerData.tg_newtuple = NULL;
- LocTriggerData.tg_trigslot = NULL;
- LocTriggerData.tg_newslot = NULL;
- LocTriggerData.tg_oldtable = NULL;
- LocTriggerData.tg_newtable = NULL;
for (i = 0; i < trigdesc->numtriggers; i++)
{
Trigger *trigger = &trigdesc->triggers[i];
@@ -2528,7 +2522,7 @@ ExecBRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
HeapTuple newtuple = NULL;
bool should_free;
- TriggerData LocTriggerData;
+ TriggerData LocTriggerData = {0};
int i;
LocTriggerData.type = T_TriggerData;
@@ -2536,12 +2530,6 @@ ExecBRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
TRIGGER_EVENT_ROW |
TRIGGER_EVENT_BEFORE;
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
- LocTriggerData.tg_trigtuple = NULL;
- LocTriggerData.tg_newtuple = NULL;
- LocTriggerData.tg_trigslot = NULL;
- LocTriggerData.tg_newslot = NULL;
- LocTriggerData.tg_oldtable = NULL;
- LocTriggerData.tg_newtable = NULL;
for (i = 0; i < trigdesc->numtriggers; i++)
{
Trigger *trigger = &trigdesc->triggers[i];
@@ -2610,7 +2598,7 @@ ExecIRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
HeapTuple newtuple = NULL;
bool should_free;
- TriggerData LocTriggerData;
+ TriggerData LocTriggerData = {0};
int i;
LocTriggerData.type = T_TriggerData;
@@ -2618,12 +2606,6 @@ ExecIRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
TRIGGER_EVENT_ROW |
TRIGGER_EVENT_INSTEAD;
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
- LocTriggerData.tg_trigtuple = NULL;
- LocTriggerData.tg_newtuple = NULL;
- LocTriggerData.tg_trigslot = NULL;
- LocTriggerData.tg_newslot = NULL;
- LocTriggerData.tg_oldtable = NULL;
- LocTriggerData.tg_newtable = NULL;
for (i = 0; i < trigdesc->numtriggers; i++)
{
Trigger *trigger = &trigdesc->triggers[i];
@@ -2675,7 +2657,7 @@ ExecBSDeleteTriggers(EState *estate, ResultRelInfo *relinfo)
{
TriggerDesc *trigdesc;
int i;
- TriggerData LocTriggerData;
+ TriggerData LocTriggerData = {0};
trigdesc = relinfo->ri_TrigDesc;
@@ -2693,12 +2675,6 @@ ExecBSDeleteTriggers(EState *estate, ResultRelInfo *relinfo)
LocTriggerData.tg_event = TRIGGER_EVENT_DELETE |
TRIGGER_EVENT_BEFORE;
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
- LocTriggerData.tg_trigtuple = NULL;
- LocTriggerData.tg_newtuple = NULL;
- LocTriggerData.tg_trigslot = NULL;
- LocTriggerData.tg_newslot = NULL;
- LocTriggerData.tg_oldtable = NULL;
- LocTriggerData.tg_newtable = NULL;
for (i = 0; i < trigdesc->numtriggers; i++)
{
Trigger *trigger = &trigdesc->triggers[i];
@@ -2755,7 +2731,7 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
TupleTableSlot *slot = ExecGetTriggerOldSlot(estate, relinfo);
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
bool result = true;
- TriggerData LocTriggerData;
+ TriggerData LocTriggerData = {0};
HeapTuple trigtuple;
bool should_free = false;
int i;
@@ -2794,12 +2770,6 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
TRIGGER_EVENT_ROW |
TRIGGER_EVENT_BEFORE;
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
- LocTriggerData.tg_trigtuple = NULL;
- LocTriggerData.tg_newtuple = NULL;
- LocTriggerData.tg_trigslot = NULL;
- LocTriggerData.tg_newslot = NULL;
- LocTriggerData.tg_oldtable = NULL;
- LocTriggerData.tg_newtable = NULL;
for (i = 0; i < trigdesc->numtriggers; i++)
{
HeapTuple newtuple;
@@ -2872,7 +2842,7 @@ ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
{
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
TupleTableSlot *slot = ExecGetTriggerOldSlot(estate, relinfo);
- TriggerData LocTriggerData;
+ TriggerData LocTriggerData = {0};
int i;
LocTriggerData.type = T_TriggerData;
@@ -2880,12 +2850,6 @@ ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
TRIGGER_EVENT_ROW |
TRIGGER_EVENT_INSTEAD;
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
- LocTriggerData.tg_trigtuple = NULL;
- LocTriggerData.tg_newtuple = NULL;
- LocTriggerData.tg_trigslot = NULL;
- LocTriggerData.tg_newslot = NULL;
- LocTriggerData.tg_oldtable = NULL;
- LocTriggerData.tg_newtable = NULL;
ExecForceStoreHeapTuple(trigtuple, slot, false);
@@ -2924,7 +2888,7 @@ ExecBSUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
{
TriggerDesc *trigdesc;
int i;
- TriggerData LocTriggerData;
+ TriggerData LocTriggerData = {0};
Bitmapset *updatedCols;
trigdesc = relinfo->ri_TrigDesc;
@@ -2945,12 +2909,6 @@ ExecBSUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
LocTriggerData.tg_event = TRIGGER_EVENT_UPDATE |
TRIGGER_EVENT_BEFORE;
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
- LocTriggerData.tg_trigtuple = NULL;
- LocTriggerData.tg_newtuple = NULL;
- LocTriggerData.tg_trigslot = NULL;
- LocTriggerData.tg_newslot = NULL;
- LocTriggerData.tg_oldtable = NULL;
- LocTriggerData.tg_newtable = NULL;
for (i = 0; i < trigdesc->numtriggers; i++)
{
Trigger *trigger = &trigdesc->triggers[i];
@@ -3005,7 +2963,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
HeapTuple trigtuple;
bool should_free_trig = false;
bool should_free_new = false;
- TriggerData LocTriggerData;
+ TriggerData LocTriggerData = {0};
int i;
Bitmapset *updatedCols;
LockTupleMode lockmode;
@@ -3058,8 +3016,6 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
TRIGGER_EVENT_ROW |
TRIGGER_EVENT_BEFORE;
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
- LocTriggerData.tg_oldtable = NULL;
- LocTriggerData.tg_newtable = NULL;
updatedCols = GetAllUpdatedColumns(relinfo, estate);
for (i = 0; i < trigdesc->numtriggers; i++)
{
@@ -3173,7 +3129,7 @@ ExecIRUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
TupleTableSlot *oldslot = ExecGetTriggerOldSlot(estate, relinfo);
HeapTuple newtuple = NULL;
bool should_free;
- TriggerData LocTriggerData;
+ TriggerData LocTriggerData = {0};
int i;
LocTriggerData.type = T_TriggerData;
@@ -3181,8 +3137,6 @@ ExecIRUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
TRIGGER_EVENT_ROW |
TRIGGER_EVENT_INSTEAD;
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
- LocTriggerData.tg_oldtable = NULL;
- LocTriggerData.tg_newtable = NULL;
ExecForceStoreHeapTuple(trigtuple, oldslot, false);
@@ -3238,7 +3192,7 @@ ExecBSTruncateTriggers(EState *estate, ResultRelInfo *relinfo)
{
TriggerDesc *trigdesc;
int i;
- TriggerData LocTriggerData;
+ TriggerData LocTriggerData = {0};
trigdesc = relinfo->ri_TrigDesc;
@@ -3251,12 +3205,6 @@ ExecBSTruncateTriggers(EState *estate, ResultRelInfo *relinfo)
LocTriggerData.tg_event = TRIGGER_EVENT_TRUNCATE |
TRIGGER_EVENT_BEFORE;
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
- LocTriggerData.tg_trigtuple = NULL;
- LocTriggerData.tg_newtuple = NULL;
- LocTriggerData.tg_trigslot = NULL;
- LocTriggerData.tg_newslot = NULL;
- LocTriggerData.tg_oldtable = NULL;
- LocTriggerData.tg_newtable = NULL;
for (i = 0; i < trigdesc->numtriggers; i++)
{
@@ -4182,7 +4130,7 @@ AfterTriggerExecute(EState *estate,
Relation rel = relInfo->ri_RelationDesc;
AfterTriggerShared evtshared = GetTriggerSharedData(event);
Oid tgoid = evtshared->ats_tgoid;
- TriggerData LocTriggerData;
+ TriggerData LocTriggerData = {0};
HeapTuple rettuple;
int tgindx;
bool should_free_trig = false;
@@ -4191,10 +4139,6 @@ AfterTriggerExecute(EState *estate,
/*
* Locate trigger in trigdesc.
*/
- LocTriggerData.tg_trigger = NULL;
- LocTriggerData.tg_trigslot = NULL;
- LocTriggerData.tg_newslot = NULL;
-
for (tgindx = 0; tgindx < trigdesc->numtriggers; tgindx++)
{
if (trigdesc->triggers[tgindx].tgoid == tgoid)
--
2.25.0
0002-Add-tg_updatedcols-to-TriggerData.patch (text/plain, 7.0K) download
From a713a831c464cd629ffc9d8627199b9ab88a41ba Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 24 Feb 2020 10:12:10 +0100
Subject: [PATCH 2/2] Add tg_updatedcols to TriggerData
This allows a trigger function to determine for an UPDATE trigger
which columns were actually updated. This allows some optimizations
in generic trigger functions such as lo_manage and
tsvector_update_trigger.
---
contrib/lo/lo.c | 3 ++-
doc/src/sgml/trigger.sgml | 20 ++++++++++++++++++++
src/backend/commands/trigger.c | 6 ++++++
src/backend/utils/adt/tsvector_op.c | 29 +++++++++++++++++++++--------
src/include/commands/trigger.h | 1 +
5 files changed, 50 insertions(+), 9 deletions(-)
diff --git a/contrib/lo/lo.c b/contrib/lo/lo.c
index 4585923ee2..b9847081db 100644
--- a/contrib/lo/lo.c
+++ b/contrib/lo/lo.c
@@ -74,7 +74,8 @@ lo_manage(PG_FUNCTION_ARGS)
* Here, if the value of the monitored attribute changes, then the large
* object associated with the original value is unlinked.
*/
- if (newtuple != NULL)
+ if (newtuple != NULL &&
+ bms_is_member(attnum - FirstLowInvalidHeapAttributeNumber, trigdata->tg_updatedcols))
{
char *orig = SPI_getvalue(trigtuple, tupdesc, attnum);
char *newv = SPI_getvalue(newtuple, tupdesc, attnum);
diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml
index ba94acad69..bda7866ecb 100644
--- a/doc/src/sgml/trigger.sgml
+++ b/doc/src/sgml/trigger.sgml
@@ -515,6 +515,7 @@ <title>Writing Trigger Functions in C</title>
TupleTableSlot *tg_newslot;
Tuplestorestate *tg_oldtable;
Tuplestorestate *tg_newtable;
+ const Bitmapset *tg_updatedcols;
} TriggerData;
</programlisting>
@@ -757,6 +758,25 @@ <title>Writing Trigger Functions in C</title>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><structfield>tg_updatedcols</structfield></term>
+ <listitem>
+ <para>
+ For <literal>UPDATE</literal> triggers, a bitmap set indicating the
+ columns that were updated by the triggering command. Generic trigger
+ functions can use this to optimize actions by not having to deal with
+ columns that were not changed.
+ </para>
+
+ <para>
+ As an example, to determine whether a column with attribute number
+ <varname>attnum</varname> (1-based) is a member of this bitmap set,
+ call <literal>bms_is_member(attnum -
+ FirstLowInvalidHeapAttributeNumber,
+ trigdata->tg_updatedcols))</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</para>
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 26593576fd..9bb5afc8d9 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2909,6 +2909,7 @@ ExecBSUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
LocTriggerData.tg_event = TRIGGER_EVENT_UPDATE |
TRIGGER_EVENT_BEFORE;
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
+ LocTriggerData.tg_updatedcols = updatedCols;
for (i = 0; i < trigdesc->numtriggers; i++)
{
Trigger *trigger = &trigdesc->triggers[i];
@@ -3017,6 +3018,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
TRIGGER_EVENT_BEFORE;
LocTriggerData.tg_relation = relinfo->ri_RelationDesc;
updatedCols = GetAllUpdatedColumns(relinfo, estate);
+ LocTriggerData.tg_updatedcols = updatedCols;
for (i = 0; i < trigdesc->numtriggers; i++)
{
Trigger *trigger = &trigdesc->triggers[i];
@@ -3573,6 +3575,7 @@ typedef struct AfterTriggerSharedData
Oid ats_relid; /* the relation it's on */
CommandId ats_firing_id; /* ID for firing cycle */
struct AfterTriggersTableData *ats_table; /* transition table access */
+ Bitmapset *ats_modifiedcols; /* modified columns */
} AfterTriggerSharedData;
typedef struct AfterTriggerEventData *AfterTriggerEvent;
@@ -4272,6 +4275,8 @@ AfterTriggerExecute(EState *estate,
LocTriggerData.tg_event =
evtshared->ats_event & (TRIGGER_EVENT_OPMASK | TRIGGER_EVENT_ROW);
LocTriggerData.tg_relation = rel;
+ if (TRIGGER_FOR_UPDATE(LocTriggerData.tg_trigger->tgtype))
+ LocTriggerData.tg_updatedcols = evtshared->ats_modifiedcols;
MemoryContextReset(per_tuple_context);
@@ -5959,6 +5964,7 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
new_shared.ats_table = transition_capture->tcs_private;
else
new_shared.ats_table = NULL;
+ new_shared.ats_modifiedcols = modifiedCols;
afterTriggerAddEvent(&afterTriggers.query_stack[afterTriggers.query_depth].events,
&new_event, &new_shared);
diff --git a/src/backend/utils/adt/tsvector_op.c b/src/backend/utils/adt/tsvector_op.c
index cab6874e70..784132aecf 100644
--- a/src/backend/utils/adt/tsvector_op.c
+++ b/src/backend/utils/adt/tsvector_op.c
@@ -2416,6 +2416,7 @@ tsvector_update_trigger(PG_FUNCTION_ARGS, bool config_column)
bool isnull;
text *txt;
Oid cfgId;
+ bool update_needed;
/* Check call context */
if (!CALLED_AS_TRIGGER(fcinfo)) /* internal error */
@@ -2428,9 +2429,15 @@ tsvector_update_trigger(PG_FUNCTION_ARGS, bool config_column)
elog(ERROR, "tsvector_update_trigger: must be fired BEFORE event");
if (TRIGGER_FIRED_BY_INSERT(trigdata->tg_event))
+ {
rettuple = trigdata->tg_trigtuple;
+ update_needed = true;
+ }
else if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
+ {
rettuple = trigdata->tg_newtuple;
+ update_needed = false; /* computed below */
+ }
else
elog(ERROR, "tsvector_update_trigger: must be fired for INSERT or UPDATE");
@@ -2518,6 +2525,9 @@ tsvector_update_trigger(PG_FUNCTION_ARGS, bool config_column)
errmsg("column \"%s\" is not of a character type",
trigger->tgargs[i])));
+ if (bms_is_member(numattr - FirstLowInvalidHeapAttributeNumber, trigdata->tg_updatedcols))
+ update_needed = true;
+
datum = SPI_getbinval(rettuple, rel->rd_att, numattr, &isnull);
if (isnull)
continue;
@@ -2530,16 +2540,19 @@ tsvector_update_trigger(PG_FUNCTION_ARGS, bool config_column)
pfree(txt);
}
- /* make tsvector value */
- datum = TSVectorGetDatum(make_tsvector(&prs));
- isnull = false;
+ if (update_needed)
+ {
+ /* make tsvector value */
+ datum = TSVectorGetDatum(make_tsvector(&prs));
+ isnull = false;
- /* and insert it into tuple */
- rettuple = heap_modify_tuple_by_cols(rettuple, rel->rd_att,
- 1, &tsvector_attr_num,
- &datum, &isnull);
+ /* and insert it into tuple */
+ rettuple = heap_modify_tuple_by_cols(rettuple, rel->rd_att,
+ 1, &tsvector_attr_num,
+ &datum, &isnull);
- pfree(DatumGetPointer(datum));
+ pfree(DatumGetPointer(datum));
+ }
return PointerGetDatum(rettuple);
}
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index 5d69192643..a40ddf5db5 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -39,6 +39,7 @@ typedef struct TriggerData
TupleTableSlot *tg_newslot;
Tuplestorestate *tg_oldtable;
Tuplestorestate *tg_newtable;
+ const Bitmapset *tg_updatedcols;
} TriggerData;
/*
--
2.25.0
view thread (5+ messages) latest in thread
Message-ID: <11c5f156-67a9-0fb5-8200-2a8018eb2e0c@2ndquadrant.com>
Permalink: ../11c5f156-67a9-0fb5-8200-2a8018eb2e0c@2ndquadrant.com/
Also on: postgresql.org/message-id/11c5f156-67a9-0fb5-8200-2a8018eb2e0c@2ndquadrant.com
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter.eisentraut@2ndquadrant.com
Subject: Re: allow trigger to get updated columns
In-Reply-To: <11c5f156-67a9-0fb5-8200-2a8018eb2e0c@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox