Re: [BUG] pg9.4.10 Logical decoding did not get the correct oldtuplelen

On 2017-01-05 13:09:28 +0900, Michael Paquier wrote:
> On Sun, Jan 1, 2017 at 9:06 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
> > OK, I can see the problem. And I can see as well that things have been
> > tackled in this area with 9.5 thanks to commit 2c03216d...
>
> (Adding Andres in CC for some input on the matter)

Thanks.

> Looking at that... As HeapTupleData->t_len is uint32, it is really an
> oversight in the 9.4 WAL logging machinery that xl_heap_header_len
> uses uint16.

Not generally, no. For new tuples and such it's perfectly fine - they
can't be bigger than MaxHeapTupleSize. It's just for the old-tuple where
(for FULL), we inline toasted columns.

> If xl_heap_update->flags still had a bit free, we could
> have for example use XLOG_HEAP_CONTAINS_OLD_TUPLE to track the
> existing short version of xl_heap_header_len and add a new bit for a
> "long" version where a version of xl_heap_header_len including uint32
> as t_len is read. This would preserve replay from doing stupid things
> for existing 9.4 deployments and allow new records to work fully. But
> there are no bits free here. As well as there are no bits for
> XLOG_HEAP[1|2]. Andres, perhaps you have an idea?

> Looking at this code, It is worth noting that
> XLOG_HEAP_CONTAINS_OLD_TUPLE and XLOG_HEAP_CONTAINS_OLD_KEY are always
> used just for XLOG_HEAP_CONTAINS_OLD, so we could merge them and have
> a single flag to do the whole thing, the parent's replica identity
> value being here to track if a full version of the old tuple is logged
> or not.

That sounds like a bad idea to me. But I don't have one a lot better,
and as you say the knowledge is currently not required. If both bits
are set it's a 32bit len, otherwise 16. Yay.

Andres