On Fri, Jul 30, 2021 at 9:32 AM Peter Smith <smithpb2...@gmail.com> wrote: > > Please find attached the latest patch set v100* >
Few minor comments: 1. CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, two_phase = true); \dRs+ + --fail - alter of two_phase option not supported. ALTER SUBSCRIPTION regress_testsub SET (two_phase = false); Spurious line addition. 2. +TransactionId +logicalrep_read_stream_prepare(StringInfo in, LogicalRepPreparedTxnData *prepare_data) +{ + logicalrep_read_prepare_common(in, "stream prepare", prepare_data); + + return prepare_data->xid; +} There is no need to return TransactionId separately. The caller can use from prepare_data, if required. 3. extern void logicalrep_read_stream_abort(StringInfo in, TransactionId *xid, TransactionId *subxid); +extern void logicalrep_write_stream_prepare(StringInfo out, ReorderBufferTXN *txn, + XLogRecPtr prepare_lsn); +extern TransactionId logicalrep_read_stream_prepare(StringInfo in, + LogicalRepPreparedTxnData *prepare_data); + + Keep the order of declarations the same as its definitions in proto.c which means move these after logicalrep_read_rollback_prepared() and be careful about extra blank lines. -- With Regards, Amit Kapila.