Re: [PATCH v1] [BUG #19516] Skip whole-row projection shortcut for OLD/NEW returning type

Good point; the patch I sent is a minimal bandaid, while the actual root
cause is GetNSItemByRangeTablePosn not handling varreturningtype.

> So I think a better solution would be to add a new function
GetNSItemByVar(), similar to GetNSItemByRangeTablePosn()

Agreed that this is a better solution than the present patch.

That said what do you think about retrofitting GetNSItemByRangeTablePosn to
accept VarReturningType too (other callers can pass VAR_RETURNING_DEFAULT)?

Thanks,
Marko

On Wed, Jun 10, 2026 at 1:54 PM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
wrote:

> On Wed, 10 Jun 2026 at 11:48, Marko Grujic
> <marko(dot)grujic(at)enterprisedb(dot)com> wrote:
> >
> > Hi, submitting a patch for bug #19516 (held in moderation queue atm).
> >
> > Looks like the root cause is that the shortcut taken in
> ParseComplexProjection
> > loses the `varreturningtype` flag, causing it to default to
> `VAR_RETURNING_DEFAULT`.
> >
> > Consequently, the default RETURNING behavior is applied, which is
> strictly wrong
> > when a user typed (old).col on INSERT/UPDATE or (new).col on DELETE.
> >
> > To fix this simply skip the shortcut when varreturningtype is set to
> VAR_RETURNING_OLD/VAR_RETURNING_NEW.
>
> Nice catch!
>
> I actually think that the root cause of the problem is
> ParseComplexProjection()'s use of GetNSItemByRangeTablePosn(), which
> returns the wrong nsitem because it only compares varno and
> varlevelsup, not varreturningtype.
>
> So I think a better solution would be to add a new function
> GetNSItemByVar(), similar to GetNSItemByRangeTablePosn() except that
> it would take a Var and return the matching nsitem, taking into
> account varreturningtype. Then ParseComplexProjection() could use that
> to return a Var with varreturningtype set correctly.
>
> This makes me wonder if there are any other users of
> GetNSItemByRangeTablePosn() that have a similar problem.
>
> Regards,
> Dean
>