Re: fast defaults in heap_getattr vs heap_deform_tuple

Hi,

On 2019-02-01 08:24:04 -0800, Andres Freund wrote:
> While working on the patch to slotify trigger.c I got somewhat confused
> by the need to expand tuples in trigger.c:
> 
> static HeapTuple
> GetTupleForTrigger(EState *estate,
>                    EPQState *epqstate,
>                    ResultRelInfo *relinfo,
>                    ItemPointer tid,
>                    LockTupleMode lockmode,
>                    TupleTableSlot **newSlot)
> {
> ...
>     if (HeapTupleHeaderGetNatts(tuple.t_data) < relation->rd_att->natts)
>         result = heap_expand_tuple(&tuple, relation->rd_att);
>     else
>         result = heap_copytuple(&tuple);
>     ReleaseBuffer(buffer);
> 
> There's no explanation why GetTupleForTrigger() needs expanding tuples,
> but most other parts of postgres don't. Normally deforming ought to take
> care of expanding missing attrs.
> 
> As far as I can tell, the reason that it's needed is that heap_gettar()
> wasn't updated along the rest of the functions. heap_deform_tuple(),
> heap_attisnull() etc look for missing attrs, but heap_getattr() doesn't.
> 
> That, to me, makes no sense. The reason that we see a difference in
> regression test output is that composite_to_json() uses heap_getattr(),
> if it used heap_deform_tuple (which'd be considerably faster), we'd get
> the default value.
> 
> Am I missing something?

This breaks HOT (and probably also foreign keys), when fast default
columns are set to NULL, because HeapDetermineModifiedColumns() gets the
values with heap_getattr(), which returns a spurious NULL for the old
value (instead of the fast default value). That then would compare equal
to the new column value set to NULL.

Greetings,

Andres Freund