Hi,
> Thanks for the comments. They look good to me and I have addressed them
> in V4 patch.
Thanks for updating the patch. I have few more cosmetic comments.
01. BuildOldKeyTuple
```
- * Build a heap tuple representing the configured REPLICA IDENTITY to represent
- * the old tuple in an UPDATE or DELETE.
+ * Build the old-key tuple to be WAL-logged for an UPDATE or DELETE, holding
+ * the old tuple's replica identity column values (or the whole old tuple,
+ * flattened, for REPLICA IDENTITY FULL).
```
Should we also clarify that non-RI can be included? Like:
```
* Build the old-key tuple to be WAL-logged for an UPDATE or DELETE, holding
* the old tuple's replica identity column values (and some row-filtered
* UPDATEs, unchanged out-of-line ones). For REPLICA IDENTITY FULL, this will
* be the whole old tuple flattened.
```
02. BuildOldKeyTuple
```
/*
* Construct a new tuple containing only the replica identity columns,
* with nulls elsewhere. While we're at it, assert that the replica
* identity columns aren't null.
*/
heap_deform_tuple(tp, desc, values, nulls);
```
I think this comment should be updated because unchanged toasted columns can be
also included. Idea:
```
/*
* Construct a new tuple containing the replica identity columns, and
when
* requested, unchanged non-replica-identity columns with external
values.
* All other columns are null. While we're at it, assert that the
replica
* identity columns aren't null.
*/
```
Best regards,
Hayato Kuroda
FUJITSU LIMITED
