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

PG Bug reporting form <noreply@postgresql.org> writes:
> Found weird postgres behavior (seems to work for >11 versions):
> 1. There is a table with data, and trigger before update for each row
> 2. Add a new column with not null default value
> 3. When trying to update the value in the old column, raise `ERROR: null
> value in column violates not-null constraint`

I confirm this bug in v12 and up.  There's no visible bug in v11,
so the attrmissing feature (which was added in 11) is not solely
to blame.  It's a component of the problem, though.

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.

If you ask me, the proximate cause of this bug was the decision
to stick attrmissing info into the "constr" field of tupledescs.
That seems pretty damfool, because there are large parts of the
system that consider that the constr info is optional and can
be discarded, or never built to begin with.  Perhaps it's too late
to revisit that, but we need to take a very very hard look at an
awful lot of places if we're to stick with that design.

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).  I've got about zero faith that there aren't dozens of
similar bugs lurking, however.

            regards, tom lane