Re: [HACKERS] logical decoding of two-phase transactions

On Fri, Jul 30, 2021 at 9:32 AM Peter Smith <smithpb2250(at)gmail(dot)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.