Re: 10.0: Logical replication doesn't execute BEFORE UPDATE OF <columns> trigger

On Mon, Oct 9, 2017 at 11:13 PM, Aleksander Alekseev
<a(dot)alekseev(at)postgrespro(dot)ru> wrote:
> Hi hackers,
>
> I've found something that looks like a bug.
>
> Steps to reproduce
> ------------------
>
> There are 3 instances of PostgreSQL 10.0 - inst1, inst2 and inst3. There
> is a table `test` on every instance:
>
> ```
> CREATE TABLE test(k TEXT PRIMARY KEY, v TEXT);
> ```
>
> Both inst1 and inst2 have `allpub` publication:
>
> ```
> CREATE PUBLICATION allpub FOR ALL TABLES;
> ```
>
> ... and inst3 is subscribed for both publications:
>
> ```
> CREATE SUBSCRIPTION allsub1
> CONNECTION 'host=10.128.0.16 user=eax dbname=eax'
> PUBLICATION allpub;
>
> CREATE SUBSCRIPTION allsub2
> CONNECTION 'host=10.128.0.26 user=eax dbname=eax'
> PUBLICATION allpub;
> ```
>
> So basically it's two masters, one replica configuration. To resolve
> insert/update conflicts I've created the following triggers on inst3:
>
> ```
> CREATE OR REPLACE FUNCTION test_before_insert()
> RETURNS trigger AS $$
> BEGIN
>
> RAISE NOTICE 'test_before_insert trigger executed';
>
> IF EXISTS (SELECT 1 FROM test where k = new.k) THEN
> RAISE NOTICE 'test_before_insert trigger - merging data';
> UPDATE test SET v = v || ';' || new.v WHERE k = new.k;
> RETURN NULL;
> END IF;
>
> RETURN NEW;
>
> END
> $$ LANGUAGE plpgsql;
>
>
> CREATE OR REPLACE FUNCTION test_before_update()
> RETURNS trigger AS $$
> BEGIN
>
> RAISE NOTICE 'test_before_update trigger executed';
>
> IF EXISTS (SELECT 1 FROM test where k = new.k) THEN
> RAISE NOTICE 'test_before_update trigger - merging data';
> UPDATE test SET v = v || ';' || new.v WHERE k = new.k;
> DELETE FROM test where k = old.k;
> RETURN NULL;
> END IF;
>
> RETURN NEW;
>
> END
> $$ LANGUAGE plpgsql;
>
> create trigger test_before_insert_trigger
> before insert on test
> for each row execute procedure test_before_insert();
>
> create trigger test_before_update_trigger
> before update of k on test
> for each row execute procedure test_before_update();
>
> ALTER TABLE test ENABLE REPLICA TRIGGER test_before_insert_trigger;
> ALTER TABLE test ENABLE REPLICA TRIGGER test_before_update_trigger;
> ```
>
> The INSERT trigger works just as expected, however the UPDATE trigger
> doesn't. On inst1:
>
> ```
> insert into test values ('k1', 'v1');
> ```
>
> In inst2:
>
> ```
> insert into test values ('k4', 'v4');
> update test set k = 'k1' where k = 'k4';
> ```
>
> Now on inst3:
>
> ```
> select * from test;
> ```
>
> Expected result
> ---------------
>
> Rows are merged to:
>
> ```
> k | v
> ----+-------
> k1 | v1;v4
> ```
>
> This is what would happen if all insert/update queries would have been
> executed on one instance.
>
> Actual result
> -------------
>
> Replication fails, log contains:
>
> ```
> [3227] ERROR: duplicate key value violates unique constraint "test_pkey"
> [3227] DETAIL: Key (k)=(k1) already exists.
> [3176] LOG: worker process: logical replication worker for subscription 16402 (PID 3227) exited with exit code 1
> ```
>
> What do you think?
>

I think the cause of this issue is that the apply worker doesn't set
updatedCols of RangeTblEntry when applying updates. So TriggerEnabled
always ends up with false. I'll make a patch and submit.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center