Agreed; there are 4 callers of GetNSItemByRangeTablePosn, 3 of which are Var-shaped, so adding a new GetNSItemByVar makes sense.
> I don't like changing GetNSItemByRangeTablePosn() in back-branches
Given that the remaining caller (the MERGE parsing code) hard-codes the sublevels_up arg to 0, there's also potential to simplify GetNSItemByRangeTablePosn or even just inline it.,
but probably best to avoid for the sake of back-portability.
> This makes me wonder if there are any other users of
GetNSItemByRangeTablePosn() that have a similar problem.
> OTOH, I think ExpandRowReference() does need updating
Indeed, that seems to be the case
postgres=# insert into t values (2, 'two') returning (old).*, old.*, (new).*, new.*;
a | b | a | b | a | b | a | b
---+-----+---+---+---+-----+---+-----
2 | two | | | 2 | two | 2 | two
(1 row)
Thanks,
Marko
On Wed, 10 Jun 2026 at 13:23, Marko Grujic
<marko.grujic@enterprisedb.com> wrote:
>
> 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)?
I don't like changing GetNSItemByRangeTablePosn() in back-branches
because some external code might be using it, though I didn't find any
examples on PGXN.
Some users of GetNSItemByRangeTablePosn() look fine, so there's no
need to change them -- for example, the MERGE parsing code, which
isn't starting from a Var, and isn't processing something that could
be in a RETURNING list.
OTOH, I think ExpandRowReference() does need updating -- at least the
comment suggests that (old.*).* will go through it, rather than
ParseComplexProjection(), so wouldn't be fixed by your original patch.
The one I'm unsure about is coerce_record_to_complex(). I can't manage
to come up with an example that breaks it, but it certainly looks like
it should be updated.
Regards,
Dean
