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

Reply via email to