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 <[email protected]> wrote: > On Wed, 10 Jun 2026 at 11:48, Marko Grujic > <[email protected]> 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 >
