On Wed, Mar 2, 2022 at 1:01 PM shiy.f...@fujitsu.com <shiy.f...@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
v22-0001-Skip-empty-transactions-for-logical-replication.patch
Description: Binary data
