On Fri, Jan 16, 2026 at 7:20 PM Hayato Kuroda (Fujitsu) <[email protected]> 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
