Re: logical replication does not fire per-column triggers - Mailing list pgsql-hackers

From Euler Taveira
Subject Re: logical replication does not fire per-column triggers
Date
Msg-id CAHE3wggLA8w9Gd_Ci1O23eJ=1zEZoZ-mFUDPyXbq-=P9CQ2Swg@mail.gmail.com
Whole thread Raw
In response to logical replication does not fire per-column triggers  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Responses Re: logical replication does not fire per-column triggers  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
List pgsql-hackers
Em sex., 13 de dez. de 2019 às 10:26, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> escreveu:
>
> The logical replication subscription side does not fire per-column
> update triggers when applying an update change.  (Per-table update
> triggers work fine.)  This patch fixes that.  Should be backpatched to PG10.
>
Using the regression test example, table tab_fk_ref have columns id
and bid. If you add a trigger "BEFORE UPDATE OF bid" into subscriber
that fires on replica, it will always fire even if you are **not**
changed bid in publisher. In logical replication protocol all columns
were changed unless it is a (unchanged) TOAST column (if a column is
part of the PK/REPLICA IDENTITY we can compare both values and figure
out if the value changed, however, we can't ensure that a value
changed for the other columns -- those that are not PK/REPLICA
IDENTITY). It is clear that not firing the trigger is wrong but firing
it when you say that you won't fire it is also wrong. Whichever
behavior we choose, limitation should be documented. I prefer the
behavior that ignores "OF col1" and always fire the trigger (because
we can add a filter inside the function/procedure).

+ /* Populate updatedCols for trigger manager */
Add a comment that explains it is not possible to (always) determine
if a column changed. Hence, "OF col1" syntax will be ignored.

+ for (int i = 0; i < remoteslot->tts_tupleDescriptor->natts; i++)
+ {
+ RangeTblEntry *target_rte = list_nth(estate->es_range_table, 0);
+
It should be outside the loop.

+ if (newtup.changed)
It should be newtup.changed[i].

You should add a test that exposes "ignore OF col1" such as:

$node_publisher->safe_psql('postgres',
    "UPDATE tab_fk_ref SET id = 6 WHERE id = 1;");


--
   Euler Taveira                                   Timbira -
http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento



pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: Why is get_actual_variable_range()'s use of SnapshotNonVacuumablesafe during recovery?
Next
From: Michael Paquier
Date:
Subject: Re: psql's \watch is broken