RE: Simplify code building the LR conflict messages

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Amit Kapila' <amit(dot)kapila16(at)gmail(dot)com>
Cc: shveta malik <shveta(dot)malik(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Peter Smith <smithpb2250(at)gmail(dot)com>
Subject: RE: Simplify code building the LR conflict messages
Date: 2026-01-19 10:55:00
Message-ID: TY7PR01MB1455481E0EA3C1BEC20298302F588A@TY7PR01MB14554.jpnprd01.prod.outlook.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Amit,

> Few comments:
> =============
> 1.
> + append_tuple_value_detail(&err_detail, NULL, NULL,
> + remote_desc, search_desc);
> +
> + appendStringInfoString(&err_detail, _(".\n"));
>
> Adding full-stop along with a newline like this appears odd. I think
> we did it like this so that the internal function
> append_tuple_value_detail() doesn't decide whether the line ends. Is
> there any better way or are we okay with this kind of coding? I see
> that previously build_tuple_value_details() also appends dot when
> required. See below code:
> build_tuple_value_details()
> {
> ...
> if (tuple_value.len == 0)
> return NULL;

After considering bit more, I found "." can be appended all the cases and
\newline can be added for some cases. Added a parameter to control \newline,
and removed some termination handlings from the errdetail... function.

> 2.
> + if (key_desc)
> + pfree(key_desc);
> + if (search_desc)
> + pfree(search_desc);
> + if (local_desc)
> + pfree(local_desc);
> + if (remote_desc)
> + pfree(remote_desc);
>
> Do we need these retail pfrees and in one in obtain_tuple_values()?
> Which context is being used to allocate this memory and won't it reset
> after applying or logging this change?

I checked and the context is ApplyMessageContext and it is cleared after each
replication protocol message. All pfree() are not needed anymore.

Fixed the rest and update the commit message. Please see attached.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment Content-Type Size
v6-0001-Fix-errdetail-for-logical-replication-conflict.patch application/octet-stream 36.0 KB
v6-0002-Fix-primary-error-message-for-conflicts.patch application/octet-stream 15.5 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amul Sul 2026-01-19 11:13:30 Re: pg_waldump: support decoding of WAL inside tarfile
Previous Message Jakub Wartak 2026-01-19 10:53:38 Re: pg_plan_advice