Re: [DEPARSE] EXPLAIN prints another ModifyTable's RETURNING aliases in a SubPlan

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: assam258(at)gmail(dot)com
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [DEPARSE] EXPLAIN prints another ModifyTable's RETURNING aliases in a SubPlan
Date: 2026-10-22 07:13:40
Message-ID: CAEZATCW_H+tGV=AHcft3o0LmkSy_Rh8rHyQ9xZCB=KeUkcNfdA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 22 Oct 2026 at 00:58, Henson Choi <assam258(at)gmail(dot)com> wrote:
>
> > This makes me think it is probably possible to construct scenarios
> > where a plan contains both VAR_RETURNING Vars that originated inside
> > a rule and some that originated from the calling query, in which
> > case using either level's aliases across-the-board would be wrong.
>
> Here is one. With your fragment, the EXPLAIN below prints
> "Output: n.a, n.a", although the two columns are the old and new
> values (1 and 101). Current master and Ayush's patch print
> "Output: n.a, new.a".

Yes, indeed. That's not ideal.

A worse example is this:

DROP TABLE IF EXISTS foo CASCADE;
CREATE TABLE foo (a int, b int, c int);
CREATE VIEW foo_v AS SELECT * FROM foo;
CREATE RULE foo_r AS ON INSERT TO foo_v DO INSTEAD
INSERT INTO foo VALUES (new.a, new.b, new.c)
RETURNING WITH (NEW AS x) x.a, b, c;

EXPLAIN (VERBOSE, COSTS OFF)
INSERT INTO foo_v AS new VALUES (1,2,3) RETURNING WITH (OLD AS x) a, b, x.c;

Previously, this would produce "Output: a, foo.b, x.c", which is not
terrible, but fails to convey the fact that "a" is actually new.a.
However, with the proposed patch, this produces "Output: x.a, foo.b,
old.c", where that first "x.a" comes from the rule action's "x" alias,
and so actually means new.a, but a user might reasonably think it
meant old.a, since they specified OLD AS x.

On reflection, I think that the simplest solution is to leave the
rewriter code as-is, but change the new Assert()'s in get_variable()
to set refname to "old" or "new" if it's NULL:

@@ -7739,12 +7739,14 @@ get_variable(Var *var, int levelsup, boo
if (var->varreturningtype == VAR_RETURNING_OLD)
{
refname = dpns->ret_old_alias;
- Assert(refname);
+ if (!refname)
+ refname = "old";
}
else if (var->varreturningtype == VAR_RETURNING_NEW)
{
refname = dpns->ret_new_alias;
- Assert(refname);
+ if (!refname)
+ refname = "new";
}

Doing that, the query above produces "Output: new.a, foo.b, x.c",
which I think is probably the least confusing result.

Regards,
Dean

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zhijie Hou 2026-10-22 07:13:46 Re: [PG19] Promotion hangs behind a backend blocked on a stuck client
Previous Message Chao Li 2026-10-22 07:03:43 Re: [PATCH] Keep git-external-diff usage text from executing