Re: Adding OLD/NEW support to RETURNING

On Sat, 30 Mar 2024 at 15:31, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
>
> Rebased version attached, on top of 0294df2f1f (MERGE .. WHEN NOT
> MATCHED BY SOURCE), with a few additional tests. No code changes, just
> keeping it up to date.
>

New version attached, rebased following the revert of 87985cc925, but
also with a few other changes:

I've added a note to rules.sgml explaining how this interacts with rules.

I've redone the way old/new system attributes are evaluated -- the
previous code changed slot_getsysattr() to try to decide when to
return NULL, but that didn't work correctly if the CTID was invalid
but non-NULL, something I hadn't anticipated, but which shows up in
the new tests added by 6572bd55b0. Instead, ExecEvalSysVar() now
checks if the OLD/NEW row exists, so there's no need to change
slot_getsysattr(), which seems much better.

I've added a new elog() error check to
adjust_appendrel_attrs_mutator(), similar to the existing one for
varnullingrels, to report if we ever attempt to apply a non-default
varreturningtype to a non-Var, which should never be possible, but
seems worth checking. (non-Var expressions should only occur if we've
flattened a UNION ALL query, so shouldn't apply to the target relation
of a data-modifying query with RETURNING.)

The previous patch added a new rewriter function
ReplaceReturningVarsFromTargetList() to rewrite the RETURNING list,
but that duplicated a lot of code from ReplaceVarsFromTargetList(), so
I've now just merged them together, which looks a lot neater.

Regards,
Dean