Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher - Mailing list pgsql-hackers

From Önder Kalacı
Subject Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
Date
Msg-id CACawEhWQimZyfV1LjF-djuEEawe2yqza4WkF_N5rQRYhxfnwTw@mail.gmail.com
Whole thread Raw
In response to RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher  ("houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>)
List pgsql-hackers
Hi Hou zj,  Shi-san, all


In this function, it used the local column number(keycol) to match the remote
column number(attkeys), I think it will cause problem if the column order
between pub/sub doesn't match. Like:

-------
- pub
CREATE TABLE test_replica_id_full (x int, y int);
ALTER TABLE test_replica_id_full REPLICA IDENTITY FULL;
CREATE PUBLICATION tap_pub_rep_full FOR TABLE test_replica_id_full;
- sub
CREATE TABLE test_replica_id_full (z int, y int, x int);
CREATE unique INDEX idx ON test_replica_id_full(z);
CREATE SUBSCRIPTION tap_sub_rep_full_0 CONNECTION 'dbname=postgres port=5432' PUBLICATION tap_pub_rep_full;
-------

I think we need to use the attrmap->attnums to convert the column number before
comparing. Just for reference, attach a diff(0001) which I noted down when trying to
fix the problem.

I'm always afraid of these types of last minute additions to the patch, and here we have
this issue on one of the latest addition :( 

Thanks for reporting the problem and also providing guidance on the fix. After reading
codes on attrMap and debugging this case further, I think your suggestion makes sense.

I only made some small changes, and included them in the patch.


Besides, I also look at the "WIP: Optimize for non-pkey / non-RI unique
indexes" patch, I think it also had a similar problem about the column
matching

Right, I'll incorporate this fix to that one as well.
 
. And another thing I think we can improved in this WIP patch is that
we can cache the result of IsIdxSafeToSkipDuplicates() instead of doing it for
each UPDATE, because the cost of this function becomes bigger after applying
this patch

Yes, it makes sense.
 

Thanks for Shi-san for helping to finish these fixes.

Thank you both!


Onder Kalaci 
Attachment

pgsql-hackers by date:

Previous
From: Melih Mutlu
Date:
Subject: Re: Allow logical replication to copy tables in binary format
Next
From: "shiy.fnst@fujitsu.com"
Date:
Subject: RE: Dropped and generated columns might cause wrong data on subs when REPLICA IDENTITY FULL