On Tue, Aug 11, 2026 at 5:38 PM shveta malik <[email protected]> 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
v69-0001-Implement-the-conflict-insertion-infrastructure-.patch
Description: Binary data
