Re: BUG #16644: null value for defaults in OLD variable for trigger

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