| 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
| 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 |