Re: logical replication empty transactions

On Wed, Mar 2, 2022 at 1:01 PM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:
>
> Hi,
>
> Here are some comments on the v21 patch.
>
> 1.
> +                       WalSndKeepalive(false, 0);
>
> Maybe we can use InvalidXLogRecPtr here, instead of 0.
>

Fixed.

> 2.
> +       pq_sendint64(&output_message, writePtr ? writePtr : sentPtr);
>
> Similarly, should we use XLogRecPtrIsInvalid()?

Fixed

>
> 3.
> @@ -1183,6 +1269,20 @@ pgoutput_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
>                         Assert(false);
>         }
>
> +   if (in_streaming)
> +       {
> +               /* If streaming, send STREAM START if we haven't yet */
> +               if (txndata && !txndata->sent_stream_start)
> +               pgoutput_send_stream_start(ctx, txn);
> +       }
> +       else
> +       {
> +               /* If not streaming, send BEGIN if we haven't yet */
> +               if (txndata && !txndata->sent_begin_txn)
> +               pgoutput_send_begin(ctx, txn);
> +       }
> +
> +
>         /* Avoid leaking memory by using and resetting our own context */
>         old = MemoryContextSwitchTo(data->context);
>
>
> I am not sure if it is suitable to send begin or stream_start here, because the
> row filter is not checked yet. That means, empty transactions caused by row
> filter are not skipped.
>

Moved the check down, so that row_filters are taken into account.

regards,
Ajin Cherian
Fujitsu Australia