Re: The documentation for storage type 'plain' actually allows single byte header

Hi,

On 2023-01-15 18:08:21 -0500, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2023-01-15 16:40:27 -0500, Tom Lane wrote:
> >> The documentation is correct, what is broken is the code. I'm not
> >> sure when we broke it
>
> > I've not thought through this fully. But after a first look, this might be
> > hard to fix without incuring a lot of overhead / complexity.
>
> It appeared to me that it was failing at this step in
> ExecGetInsertNewTuple:
>
> if (relinfo->ri_newTupleSlot->tts_ops != planSlot->tts_ops)
> {
> ExecCopySlot(relinfo->ri_newTupleSlot, planSlot);
> return relinfo->ri_newTupleSlot;
> }
>
> ri_newTupleSlot has the tupdesc we want, planSlot is a virtual slot
> that has the bogus tupdesc, and for some reason heap_form_tuple is
> getting called with planSlot's tupdesc not ri_newTupleSlot's.

The way we copy a slot into a heap slot is to materialize the source slot and
copy the heap tuple into target slot. Which is also what happened before the
slot type abstraction (hence the problem also existing before that was
introduced).

> I'm not quite sure if this is just a thinko somewhere or there's a
> deficiency in the design of the slot APIs.

I think it's fairly fundamental that copying between two slots assumes a
compatible tupdescs.

I think the problem is more in the determination whether we need to project,
or not (i.e. ExecInitInsertProjection()). But we can't really make a good
decision, because we just determine the types of "incoming" tuples based on
targetlists, which don't contain information about the storage type.

Greetings,

Andres Freund