Re: Fix a bug in DecodeAbort() and improve input data check on subscriber.

On Tue, Dec 7, 2021 at 6:06 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> Hi all,
>
> While updating the patch I recently posted[1] to make pg_waldump
> report replication origin ID, LSN, and timestamp, I found a bug that
> replication origin timestamp is not set in ROLLBACK PREPARED case.
> Commit 8bdb1332eb5 (CC'ed Amit) added an argument to
> ReorderBufferFinishPrepared() but in ROLLBACK PREPARED case, the
> caller specified it at the wrong position:
>
> @@ -730,6 +730,7 @@ DecodeCommit(LogicalDecodingContext *ctx,
> XLogRecordBuffer *buf,
>     if (two_phase)
>     {
>         ReorderBufferFinishPrepared(ctx->reorder, xid, buf->origptr,
> buf->endptr,
> +
> SnapBuildInitialConsistentPoint(ctx->snapshot_builder),
>                                     commit_time, origin_id, origin_lsn,
>                                     parsed->twophase_gid, true);
>     }
> @@ -868,6 +869,7 @@ DecodeAbort(LogicalDecodingContext *ctx,
> XLogRecordBuffer *buf,
>     {
>         ReorderBufferFinishPrepared(ctx->reorder, xid, buf->origptr,
> buf->endptr,
>                                     abort_time, origin_id, origin_lsn,
> +                                   InvalidXLogRecPtr,
>                                     parsed->twophase_gid, false);
>     }
>
> This affects the value of rollback_data.rollback_time on the
> subscriber, resulting in setting the wrong timestamp to both the
> replication origin timestamp and the error callback argument on the
> subscriber. I've attached the patch to fix it.
>

Thanks for the report and patches. I see this is a problem and the
first patch will fix it. I'll test the same and review another patch
as well.

-- 
With Regards,
Amit Kapila.