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