Re: Simplify code building the LR conflict messages

On Mon, Jan 19, 2026 at 10:56 AM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>

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;

appendStringInfoChar(&tuple_value, '.');
…
}

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?

3.
+static void
+obtain_tuple_values(EState

Won't it be better to name this function as get_tuple_values()?

--
With Regards,
Amit Kapila.