On Wed, Sep 30, 2020 at 11:21 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> After tracing through it, what I find is that:
>
> 1. Where ExecBRUpdateTriggers fetches the "trigtuple" (line 2695
> of trigger.c, in HEAD) what it gets is a raw copy of the old
> on-disk tuple, with natts = 1. That gets passed to the trigger
> function as OLD, but it reads correctly because it's being decoded
> with the relation's tupdesc, which has the needed info to fill in
> attrmissing columns.
>
> 2. When the trigger does "return OLD", trigtuple is what gets
> passed back, and it gets jammed into the slot that ExecUpdate
> passed to ExecBRUpdateTriggers. *That slot does not have any
> attrmissing info*. Thus, subsequent examination of the slot,
> in particular by ExecConstraints, concludes that the recently-
> added column is NULL.
>
> I've not tried to find where is the difference between 11 and
> 12 that makes it fail or not fail. It's likely some innocent
> seeming aspect of the slot management hacking that Andres
> was doing around that time. It's a bit odd though, because
> AFAICS the slot in question is going to be the result slot
> of the JunkFilter used by ExecModifyTable, and I'd rather have
> expected that that slot would never have had any constraints,
> in any version.
In v11, GetTupleForTrigger() expands the HeapTuple, with this:
/*
* While this is not necessary anymore after 297d627e, as a defense
* against C code that has not recompiled for minor releases after the
* fix, continue to expand the tuple.
*/
if (HeapTupleHeaderGetNatts(tuple.t_data) < relation->rd_att->natts)
result = heap_expand_tuple(&tuple, relation->rd_att);
else
result = heap_copytuple(&tuple);
ReleaseBuffer(buffer);
So the tuple that gets slammed into JunkFilter's result slot is an
expanded copy of the on-disk tuple, so it doesn't matter that its
TupleDesc doesn't have constraints (constr) initialized.
In v12 and further, I see that the tuple fetched by
GetTupleForTrigger() is slammed into JunkFilter's result slot
unexpanded and it remains so through ExecConstraint().
> I think we can band-aid this immediate problem by forcing
> trigger.c to materialize the "old" tuples it fetches from disk
> (or whatever needs to be done to substitute missing values into
> them).
Maybe something like the attached?
--
Amit Langote
EDB: http://www.enterprisedb.com