Re: logical replication does not fire per-column triggers

From: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
To: Euler Taveira <euler(at)timbira(dot)com(dot)br>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: logical replication does not fire per-column triggers
Date: 2019-12-16 13:37:00
Message-ID: d8bc27de-cd87-fd52-9de1-0552f37d1e5c@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2019-12-14 03:13, Euler Taveira wrote:
> 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).

There is a small difference: If the subscriber has extra columns not
present on the publisher, then a column trigger covering only columns in
published column set will not fire.

In practice, a column trigger is just an optimization. The column it is
triggering on might not have actually changed. The opposite is worse,
not firing the trigger when the column actually has changed.

> + /* 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.

done

> + 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.

fixed

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

fixed

> 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;");

done

New patch attached.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
v2-0001-Have-logical-replication-subscriber-fire-column-t.patch text/plain 4.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-12-16 13:55:32 Re: What's the best way to get flex and bison on Windows?
Previous Message Surafel Temesgen 2019-12-16 13:23:59 Re: Conflict handling for COPY FROM