Re: Confusing with commit time usage in logical decoding

Hi,

On 2016-03-01 18:09:28 +0100, Petr Jelinek wrote:
> On 01/03/16 17:57, Alvaro Herrera wrote:
> >Artur Zakirov wrote:
> >>Hello, Andres
> >>
> >>You have introduced a large replication progress tracking infrastructure
> >>last year. And there is a problem described at the link in the quote below.
> >>
> >>Attached patch fix this issue. Is this patch correct? I will be grateful if
> >>it is and if it will be committed.
> >
> >AFAICS this is clearly a bug introduced in 5aa235042:
> >
> > /* replay actions of all transaction + subtransactions in order */
> > ReorderBufferCommit(ctx->reorder, xid, buf->origptr, buf->endptr,
> >- parsed->xact_time);
> >+ commit_time, origin_id, origin_lsn);
> > }
> >
>
> Well yeah but the commit_time is set few lines above as Artur pointed out, I
> think the proposed fix is correct one.

I'd rather just initialize commit_time to parsed->xact_time.

This indeed is clearly a bug. I do wonder if anybody has a good idea
about how to add regression tests for this? It's rather annoying that
we have to suppress timestamps in the test_decoding tests, because
they're obviously not reproducible...

Andres