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
