Re: row filtering for logical replication

On Tue, Dec 21, 2021 at 5:19 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> Yeah, deforming tuples again can have a significant cost but what is
> the need to maintain tmp_new_tuple in relsyncentry. I think that is
> required in rare cases, so we can probably allocate/deallocate when
> required.

Fixed this.

>
> Few other comments:
> ==================
> 1.
> TupleTableSlot *scantuple; /* tuple table slot for row filter */
> + TupleTableSlot *new_tuple; /* slot for storing deformed new tuple
> during updates */
> + TupleTableSlot *old_tuple; /* slot for storing deformed old tuple
> during updates */
>
> I think it is better to name these as scan_slot, new_slot, old_slot to
> avoid confusion with tuples.
>

Fixed this.

> 2.
> +++ b/src/backend/replication/logical/proto.c
> @@ -19,6 +19,7 @@
> #include "replication/logicalproto.h"
> #include "utils/lsyscache.h"
> #include "utils/syscache.h"
> +#include "executor/executor.h"
>
> The include is in wrong order. We keep includes in alphabatic order.
>

Fixed this.

> 3.
> @@ -832,6 +847,7 @@ logicalrep_write_tuple(StringInfo out, Relation
> rel, HeapTuple tuple, bool binar
>
> ReleaseSysCache(typtup);
> }
> +
> }
>
> Spurious addition.
>

Fixed this.

regards,
Ajin Cherian