On Fri, Jan 16, 2026 at 7:20 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.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
