From: | Önder Kalacı <onderkalaci(at)gmail(dot)com> |
---|---|
To: | "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com> |
Cc: | "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Marco Slot <marco(dot)slot(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "wangw(dot)fnst(at)fujitsu(dot)com" <wangw(dot)fnst(at)fujitsu(dot)com> |
Subject: | Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher |
Date: | 2023-03-13 10:51:00 |
Message-ID: | CACawEhWQimZyfV1LjF-djuEEawe2yqza4WkF_N5rQRYhxfnwTw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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 | Content-Type | Size |
---|---|---|
v45_0001_use_index_on_subs_when_pub_rep_ident_full.patch | application/x-patch | 47.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | shiy.fnst@fujitsu.com | 2023-03-13 10:53:03 | RE: Dropped and generated columns might cause wrong data on subs when REPLICA IDENTITY FULL |
Previous Message | Melih Mutlu | 2023-03-13 10:29:32 | Re: Allow logical replication to copy tables in binary format |