Re: Proposal: Conflict log history table for Logical Replication

On Tue, Nov 3, 2026 at 5:38 PM Narayanan Venkateswaran
<narayananvpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> I looked at v89 of the patch, I had just one small comment (different
> from the comments posted already),
>
> In insert_conflict_log_tuple(): we have the following lines
>
> + if (remote_commit_ts > 0)
> + values[attno++] = TimestampTzGetDatum(remote_commit_ts);
> + else
> + nulls[attno++] = true;
>
> I think "if (remote_commit_ts != 0)" would be better for the following reasons,
>
> 1. build_local_conflicts_json_array() just above checks "if (conflicttuple->ts)"
> 2. The patch also has the line,
>
> + TimestampTz commit_ts; /* remote commit ts; 0 if unknown */
>
> we explicitly define 0 as the sentinel when the remote commit
> timestamp is unknown

Yeah make sense, and also for consistency with conflicttuple->ts we
can check, if (remote_commit_ts != 0) or if (remote_commit_ts)

--
Regards,
Dilip Kumar
Google