| From: | Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, assam258(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-21 20:47:26 |
| Message-ID: | CAJTYsWXdwB6sLTPm3+4PSZWk8mjYjvZFsy94VWnEJbeqOBFrFw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Wed, 21 Oct 2026 at 23:30, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> I wrote:
> > But wait a minute: there's still something pretty strange
> > going on here. In the first place, Ayush's test case doesn't
> > crash if the outer query doesn't try to use alias "new" for v:
>
> Ah, I see the cause of that part. The parser's
> transformReturningClause does this:
>
> if (qry->returningNewAlias == NULL &&
> refnameNamespaceItem(pstate, NULL, "new", -1, NULL) == NULL)
> {
> qry->returningNewAlias = "new";
> addNSItemForReturning(pstate, "new", VAR_RETURNING_NEW);
> }
>
> (and likewise for returningOldAlias). So using "new" as an
> alias in the outer query allows qry->returningNewAlias to be
> left as NULL. I guess the assumption is that if you do that
> then it's impossible to reference VAR_RETURNING_NEW explicitly?
> But then when rewriteRuleAction unconditionally copies the null
> value into the rule_action --- which does have VAR_RETURNING_NEW
> Vars --- we've got trouble.
>
> Contrarily, it's possible for the rule_action to have null
> returningOldAlias/returningNewAlias through the same mechanism.
> One regression test case that fails if I simply dike out
> rewriteRuleAction's copying is
>
> CREATE TEMP TABLE foo (f1 serial, f2 text, f3 int default 42);
> ALTER TABLE foo ADD COLUMN f4 int8 DEFAULT 99;
> CREATE RULE foo_del_rule AS ON DELETE TO foo DO INSTEAD
> UPDATE foo SET f2 = f2||' (deleted)', f3 = -1, f4 = -1 WHERE f1 = OLD.f1
> RETURNING *;
> EXPLAIN (verbose, costs off)
> DELETE FROM foo WHERE f1 = 4 RETURNING old.*,new.*, *;
>
> Here, the rule_action's aliases are both NULL, which I take to be
> because the rule mechanisms themselves set up dummy OLD and NEW RTEs.
> So if we don't supply something during rewriting then those new
> Asserts fire, as a consequence of having pushed VAR_RETURNING_OLD/NEW
> Vars down into the rule action from the outer query.
>
> 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.
> But we don't have a lot of choice, because that mixing happens in
> the rewriter and its output format doesn't support recording multiple
> sets of aliases (or does it? maybe the two sets of Vars will have
> different varlevelsup?).
AFAICS no, both come out at varlevelsup 0 with the same varno. E.g.
if the rule does RETURNING WITH (OLD AS o) o.a, t.b, an outer
UPDATE v ... RETURNING old.b shows up as o.b in EXPLAIN
> Anyway, for a back-patchable fix I favor the fragment I showed,
> which keeps the lower-level aliases if any and only pushes down
> the upper aliases if there are no aliases in the rule.
Sounds good to me, thanks for checking it out.
I tried it out and it fixes both the NEW and OLD cases..
Regards,
Ayush
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-10-21 21:03:14 | Re: [DEPARSE] EXPLAIN prints another ModifyTable's RETURNING aliases in a SubPlan |
| Previous Message | Baji Shaik | 2026-10-21 20:42:28 | Fix rescan costing of single-batch hash joins |