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
