| From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, saurabh singh <saurabh(dot)singh214(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Proposal: Conflict log history table for Logical Replication |
| Date: | 2026-10-11 09:36:41 |
| Message-ID: | CAFiTN-uxsFDUGfuf1e6D1dPGfJYv2Gm_dOkVYohKuX-Uxm3_aA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, Oct 9, 2026 at 11:17 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Oct 9, 2026 at 4:08 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Fri, Oct 9, 2026 at 2:51 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > Okay, I see the problem but I wonder why you didn't consider using
> > > rd_opcintype stored in index relation to solve this problem. I have
> > > tried the attached atop v79-0001 and v79-0002, manually executed the
> > > test you shared in this email and it fixed the issue for me.
> >
> > Yeah you can do that, but if we directly fetch the heap attribute and
> > then value from the slot like I have done in my 0003 patch you can get
> > rid of all the processing done inside build_index_datums_from_slot()
> > i.e. calling FormIndexDatum(BuildIndexInfo) etc. If you see my patch
> > with that I am able to remove all this extra processing, I am sure we
> > can get rid of that by some other way as well but I think thats the
> > most obvious way of doing it.
> >
>
> I was looking from the code consistency point of view. We already used
> this idea in build_index_value_desc() code path while reporting the
> conflict in LOG, so it looks natural to use the same idea here. Also,
> as this is a path where conflict already happened and we decided to
> insert into conflict_history_table, so the performance will anyway be
> not the main concern, so we can consider this optimization separately.
Okay that makes sense. Here is the updated version of the patch
changes compared to previous version
1. Patch 0001 is the same, except the top up changes(0004) applicable
to 0001 have been merged into it.
2. Patch 0002 -> added the fix suggested by Amit and also included the
check what Shveta suggested on the output value and also removed the
check on the raw value, we may think of keeping the check on raw value
as well as an optimization but logically we are talking about the edge
cases where the some of the key column is crossing 64k limit so I
don't think we need to affect the common cases for erly detecting the
edge cases.
3. Patch 0003 is not required because we included Amit's solution in 0002
4. Patch 0004 is merged with 0001 and 0002 as applicable.
--
Regards,
Dilip Kumar
Google
| Attachment | Content-Type | Size |
|---|---|---|
| v80-0001-Implement-the-conflict-insertion-infrastructure-.patch | application/octet-stream | 81.4 KB |
| v80-0002-Don-t-use-row_to_json-or-record-oversized-values.patch | application/octet-stream | 23.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ayush Tiwari | 2026-10-11 09:54:27 | [PATCH] Clear walsender reader pointers after cleanup |
| Previous Message | Rahul | 2026-10-11 07:46:44 | Re: Wrong LSN in WAL decoding error messages |