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-761181204...@2ndquadrant.com

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 0c901266a1b40a257320166fdacaaefd00e4dbce Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@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

From a713a831c464cd629ffc9d8627199b9ab88a41ba Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@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

Reply via email to