Re: Proposal: Conflict log history table for Logical Replication

Hi Nisha,
Thank you for the patch. I applied it locally, and it compiles successfully.
I have a few suggestions for consideration for v75. Please treat these
as optional review comments;
you and the others can decide whether they are important enough to address.
Otherwise, feel free to ignore them.

> Attached are the updated patches:
> v75-001: Merged v74 patches 001 and 002 into a single patch.
> v75-002: Includes the discussed fix, along with TAP tests covering
> both cases. Also updated the documentation with an example.
>

1. Documentation:
In the example query, could the hard-coded table name:
pg_conflict.pg_conflict_log_16392
be replaced with a placeholder such as:
pg_conflict_log_<subid>
which is used at other places in doc.

2. Documentation:
The documentation says that values larger than 1 kB are omitted.
However, the implementation
measures raw detoasted storage size rather than textual output size.
This distinction may
matter for custom types, arrays, compressed values, and JSON-like types.

3. Code:
In:
+ return PointerGetDatum(cstring_to_text_with_len(result.data, result.len));

`result.data` does not appear to be released after the text datum is created.
Could this accumulate memory when multiple conflicts occur in one
apply transaction?

4. Code:
The name `tuple_table_slot_to_indextup_json` was not immediately clear to me.
Since the function serializes index key values rather than an index tuple,
would something like `tuple_table_slot_to_index_key_json` be more descriptive?

5. Code:
Is the following variable necessary? It appears not to be used currently:
+ bool typisvarlena;

Thanks again for the work.

Regards,
Kedar