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

On Sat, Jul 31, 2021 at 9:36 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Jul 30, 2021 at 9:32 AM Peter Smith <smithpb2250@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.
>

OK. Fixed in v101.

> 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.
>

OK. Modified in v101

> 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.
>

OK. Reordered in v101.

------
Kind Regards,
Peter Smith.
Fujitsu Australia