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.