On Mon, Jan 31, 2022 at 6:18 PM Ajin Cherian <itsa...@gmail.com> wrote: >
Few comments: ============= 1. Is there any particular why the patch is not skipping empty xacts for streaming (in-progress) transactions as noted in the commit message as well? 2. +static void +pgoutput_begin(LogicalDecodingContext *ctx, ReorderBufferTXN *txn) +{ bool send_replication_origin = txn->origin_id != InvalidRepOriginId; + PGOutputTxnData *txndata = (PGOutputTxnData *) txn->output_plugin_private; + + Assert(txndata); I think here you can add an assert for sent_begin_txn to be always false? 3. +/* + * Send BEGIN. + * This is where the BEGIN is actually sent. This is called + * while processing the first change of the transaction. + */ Have an empty line between the first two lines to ensure consistency with nearby comments. Also, the formatting of these lines appears awkward, either run pgindent or make sure lines are not too short. 4. Do we really need to make any changes in PREPARE transaction-related functions if can't skip in that case? I think you can have a check if the output plugin private variable is not set then ignore special optimization for sending begin. -- With Regards, Amit Kapila.