Re: Proposal: Conflict log history table for Logical Replication

On Tue, Aug 11, 2026 at 5:38 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> A few comments:
>
> 1)
> + or both. Note that only resolved conflicts (logged at
> <literal>LOG</literal>
> + level where the apply transaction continues) are recorded in the
> conflict log
> + table. Conflicts that raise an <literal>ERROR</literal> halt
> replication and
> + are always reported exclusively to the server log.
>
> I think this is not needed in the Conflict-logging introduction
> section itself. It is more suited in 'table-logging' section. We can
> keep it as a NOTE (<note> tag) so that it catches reader’s eye. We can
> add it before 'The conflict log table serves as an audit trail'
> paragraph.

Done

>
> 2)
> +conflict_log_insert_errcontext(void *arg)
>
> I think we don't need this callbackfunction and logic around it anymore.

Fixed

> 3)
> In tuple_table_slot_to_indextup_json(), we first call
> BlessTupleDesc(tupdesc), but then free the tuple descriptor with
> FreeTupleDesc(tupdesc) before calling row_to_json().
>
> I am not entirely sure whether row_to_json() may need the tuple
> descriptor here, but it seems safer to keep the descriptor alive until
> after row_to_json() has consumed the record Datum. heap_form_tuple()
> and heap_copy_tuple_as_datum() also use the tuple descriptor while
> constructing the Datum;see heap_fill_tuple() uses the tuple
> descriptor's attributes. IMO, to be on the safe side, we could do:
>
> /* Convert to a JSON datum. */
> datum = DirectFunctionCall1(row_to_json, datum);
> FreeTupleDesc(tupdesc);
> return datum;

Changed.

--
Regards,
Dilip Kumar
Google