Re: Simplify code building the LR conflict messages

On Fri, Jan 16, 2026 at 7:20 PM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Dear hackers,
>
> Thanks for giving comments!
>
> I intended to avoid constructing messages and it added some complexity. I
> understood codes were hard to accept, attached is a simplified version. All info
> like key, local tuple, remote tuple, and replica identity are shown after the
> colon with the order. I also tried to reduce changes as much as possible.
> Additionally, changes in ReportApplyConflict() are moved to 0002.
>
> BTW, I found that in CT_UPDATE_ORIGIN_DIFFERS and CT_DELETE_ORIGIN_DIFFERS cases
> assume localts is available because it can happen only when the commit_timestamp
> is tracked. I think the assumption is OK, but can we add Assert(localts) for them?
> If acceptable I can add up more top-up patch.

+1, makes sense to me.

Please find a few trivial comments:

1)
append_tuple_value_detail():
+/*
+ * Helper function to build the additional details for conflicting key,
+ * existing local row, remote row, and replica identity columns.
+ */
we can get rid of 'existing' from this comment too.

Same is true for header-comment of obtain_tuple_values().

2)

+ bool comma_needed = false;

Shall we change above to 'bool first = true'. 'comma_needed' looks
slightly odd.

3)
errdetail_apply_conflict()

- *
- * The DETAIL line comprises of two parts:
- * 1. Explanation of the conflict type, including the origin and commit
- * timestamp of the existing local row.
- * 2. Display of conflicting key, existing local row, remote new row, and
- * replica identity columns, if any. The remote old row is excluded as its
- *

Why did we remove this part? IMO, it still makes sense.

thanks
Shveta