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

Henson Choi <assam258@gmail.com> writes:

The aliases are not stored in the Vars, which only say whether they
are OLD or NEW (varreturningtype). ruleutils.c keeps the names of
the ModifyTable it is looking at in the deparse namespace, and
get_variable() prints them. set_deparse_context_plan(), which
EXPLAIN calls for each node it prints, set them when that node was a
ModifyTable. But to print a Param of a SubPlan, get_parameter()
moves to the ancestor ModifyTable with push_ancestor_plan(), and the
set_deparse_plan() that it calls left the aliases alone. They were
still those of the last ModifyTable EXPLAIN had printed, here the
CTE's.

Yeah, that should be done in set_deparse_plan(). But I think we
can make this tighter: RETURNING variables should only appear in
ModifyTable nodes. So rather than allow the behavior to be
plan-traversal-order-dependent, I made the set_deparse_plan()
addition look like

+	if (IsA(plan, ModifyTable))
+	{
+		dpns->ret_old_alias = ((ModifyTable *) plan)->returningOldAlias;
+		dpns->ret_new_alias = ((ModifyTable *) plan)->returningNewAlias;
+	}
+	else
+		dpns->ret_old_alias = dpns->ret_new_alias = NULL;

and added Asserts in get_variable that when it consults those fields,
it doesn't get a NULL. Pushed with those changes.

regards, tom lane

Hi,

While looking at 1999fe227df, I noticed that the new Assert(refname)
in get_variable() can be hit through a rule (v v edge case, fuzz testing):

CREATE TABLE t (a int);
CREATE VIEW v AS SELECT * FROM t;
CREATE RULE v_ins AS ON INSERT TO v DO INSTEAD
INSERT INTO t VALUES (new.a) RETURNING WITH (NEW AS n) n.a;
EXPLAIN (VERBOSE, COSTS OFF)
INSERT INTO v AS new VALUES (1) RETURNING *;

TRAP: failed Assert("refname"), File: "ruleutils.c", Line: 7747

IIUC, the INSERT already uses "new" as its alias, so it has no NEW
alias for RETURNING, and rewriteRuleAction() then copies that empty
alias over the rule's "n". The same seems to happen for OLD, with an
UPDATE rule and UPDATE v AS old.

The attached patch just keeps the rule's alias in that case, and
EXPLAIN then shows "Output: n.a". Thoughts?

Regards,
Ayush

Ayush Tiwari <ayushtiwari.slg01@gmail.com> writes:

While looking at 1999fe227df, I noticed that the new Assert(refname)
in get_variable() can be hit through a rule (v v edge case, fuzz testing):

Ah, I wondered if those Asserts were too aggressive.

The attached patch just keeps the rule's alias in that case, and
EXPLAIN then shows "Output: n.a". Thoughts?

We could just drop the Asserts and thereby return to the previous
behavior. But perhaps your proposal is better. Dean, any thoughts?

regards, tom lane

On Wed, 21 Oct 2026 at 15:46, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Ayush Tiwari <ayushtiwari.slg01@gmail.com> writes:

While looking at 1999fe227df, I noticed that the new Assert(refname)
in get_variable() can be hit through a rule (v v edge case, fuzz testing):

Ah, I wondered if those Asserts were too aggressive.

The attached patch just keeps the rule's alias in that case, and
EXPLAIN then shows "Output: n.a". Thoughts?

We could just drop the Asserts and thereby return to the previous
behavior. But perhaps your proposal is better. Dean, any thoughts?

I think just dropping the Asserts is the better solution, because
showing "Output: n.a" in the EXPLAIN output is likely to lead to
confusion, since there was no relation with alias "n" in the
triggering query, and "n" isn't mentioned anywhere else in the EXPLAIN
output. The user would have to lookup the rule definition to figure
out what "n" means.

Regards,
Dean

Dean Rasheed <dean.a.rasheed@gmail.com> writes:

On Wed, 21 Oct 2026 at 15:46, Tom Lane <tgl@sss.pgh.pa.us> wrote:

We could just drop the Asserts and thereby return to the previous
behavior. But perhaps your proposal is better. Dean, any thoughts?

I think just dropping the Asserts is the better solution, because
showing "Output: n.a" in the EXPLAIN output is likely to lead to
confusion, since there was no relation with alias "n" in the
triggering query, and "n" isn't mentioned anywhere else in the EXPLAIN
output. The user would have to lookup the rule definition to figure
out what "n" means.

Good point. I'll make it so.

regards, tom lane

Hi,

On Wed, 21 Oct 2026 at 23:30, Tom Lane <tgl@sss.pgh.pa.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

On Wed, 21 Oct 2026 at 19:00, Tom Lane <tgl@sss.pgh.pa.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?

Yes, that's intentional. If a query has some relation with alias
"new", and you don't specify RETURNING WITH (NEW AS ...), then there
is no way to get a non-default VarReturningType, because "new.colname"
is just a regular reference to that column of the relation with alias
"new".

But then when rewriteRuleAction unconditionally copies the null
value into the rule_action --- which does have VAR_RETURNING_NEW
Vars --- we've got trouble.

Right.

(BTW, that can only happen if the rule action's RETURNING list
specifies an alias for NEW and contains Vars using it, since otherwise
NEW is the special pseudo-relation representing the values from the
triggering query, and "new.colname" has VAR_RETURNING_DEFAULT.)

Contrarily, it's possible for the rule_action to have null
returningOldAlias/returningNewAlias through the same mechanism.

Right. In fact, that's likely the most common case, and it's what you
get if you follow the recommendation in the docs of not using OLD/NEW
in the RETURNING list of a rule action, which then avoids all this
confusion.

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.

Yep.

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?).

No, they'll be indistinguishable.

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.

That is consistent with the way a Vars' varReturningType is propagated
-- if the rule action's RETURNING list causes a Var to have a
non-default VarRetruningType, that cannot be overridden by the
triggering query. Otherwise, the VarReturningType from the triggering
query is used, as in the example above. This is why the docs recommend
using "RETURNING *" in a rule action, rather than specifying OLD/NEW
-- so the triggering query has control over whether to return old or
new, and all the confusion is avoided. With hindsight, perhaps it
would have been better to simply forbid RETURNING WITH (OLD/NEW AS
...) in a rule action.

Regards,
Dean

Dean Rasheed <dean.a.rasheed@gmail.com> writes:

On Wed, 21 Oct 2026 at 19:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:

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.

That is consistent with the way a Vars' varReturningType is propagated
-- if the rule action's RETURNING list causes a Var to have a
non-default VarRetruningType, that cannot be overridden by the
triggering query.

So just to clarify, you now agree with that fix instead of dropping
the new Asserts?

regards, tom lane

On Wed, 21 Oct 2026 at 22:15, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Dean Rasheed <dean.a.rasheed@gmail.com> writes:

On Wed, 21 Oct 2026 at 19:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:

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.

That is consistent with the way a Vars' varReturningType is propagated
-- if the rule action's RETURNING list causes a Var to have a
non-default VarRetruningType, that cannot be overridden by the
triggering query.

So just to clarify, you now agree with that fix instead of dropping
the new Asserts?

Yes, I'm fine either way actually, because I think it's not
particularly likely to come up in practice.

Regards,
Dean

On Wed, 21 Oct 2026 at 22:43, Tom Lane <tgl@sss.pgh.pa.us> wrote:

(If this is in fact rare, and it probably is given that the manual
deprecates RETURNING WITH in rules, then it's likely not worth
the trouble to try to fix the query representation to keep the
aliases separate.)

Agreed.

Actually, I don't think that this is a problem with the query
representation. It's a problem with the output text format used in
EXPLAIN. Changing the query representation won't make that problem go
away -- a query like this may end up with a returning list containing
a mix of Var's with different VarReturningType's some of which came
from the triggering query, and some from the rule action, and simply
outputting "alias.colname" isn't enough to distinguish between them. I
think you'd have to output something like "new(tbl.colname)", which
would be different from "new.column" and "tbl.colname",

Regards,
Dean

Hi Tom, Dean,

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

CREATE TABLE t (a int, b int);
INSERT INTO t VALUES (1, 10);
CREATE VIEW v AS SELECT * FROM t;
CREATE RULE v_upd AS ON UPDATE TO v DO INSTEAD
UPDATE t SET a = new.a, b = new.b WHERE a = old.a
RETURNING WITH (NEW AS n) n.a, n.b;
EXPLAIN (VERBOSE, COSTS OFF)
UPDATE v SET a = a + 100
RETURNING WITH (OLD AS n) n.a AS old_a, new.a AS new_a;

The calling query names OLD "n" and the rule names NEW "n", so with
the rule's aliases preferred the two sides get the same name.

Best regards,
Henson

On Thu, 22 Oct 2026 at 00:58, Henson Choi <assam258@gmail.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

Hi Dean,

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.

I built master without the Assert()s, Tom's fragment and your
change, and tried them on the examples in this thread and a few
more. Neither fix gives what I would call the best result. But
for now, choosing the second best is the best we can do, and
I think that is Tom's fragment.

RETURNING WITH is there to avoid name clashes. When the rule
and the query use the same name, no fix can tell them apart
("x" in your example, "n" in mine upthread). What matters is
whether the output stays distinguishable when the user avoids
clashes. Tom's fragment only uses names that the rule action or
the query actually has, so it does.

For example, with the rule and the query both using "y":

CREATE TABLE t (a int, b int);
CREATE VIEW v AS SELECT * FROM t;
CREATE RULE r AS ON UPDATE TO v DO INSTEAD
UPDATE t SET a = new.a, b = new.b WHERE a = old.a
RETURNING WITH (OLD AS y) y.a, t.b;
EXPLAIN (VERBOSE, COSTS OFF)
UPDATE v AS old SET a = old.a + 100
RETURNING WITH (NEW AS y) y.a AS new_a, a, b;

without the Assert()s: y.a, a, t.b
Tom's fragment: y.a, y.a, t.b
your change: y.a, old.a, t.b

and with "y1" in the rule and "y2" in the query instead:

CREATE OR REPLACE RULE r AS ON UPDATE TO v DO INSTEAD
UPDATE t SET a = new.a, b = new.b WHERE a = old.a
RETURNING WITH (OLD AS y1) y1.a, t.b;
EXPLAIN (VERBOSE, COSTS OFF)
UPDATE v AS old SET a = old.a + 100
RETURNING WITH (NEW AS y2) y2.a AS new_a, a, b;

without the Assert()s: y2.a, a, t.b
Tom's fragment: y2.a, y1.a, t.b
your change: y2.a, old.a, t.b

Tom's fragment cannot tell the two "y"s apart, but once the user
avoids the clash it prints the names as written. Your change
prints "old.a" either way, and "old" is what the query calls the
view, not a name anyone gave to OLD.

The cost is that the rule's alias wins when both the rule and
the query name OLD or NEW, so a user who names them in the
query can see other names in EXPLAIN. As Tom said, the manual
already steers rule actions away from OLD and NEW in RETURNING,
so I think that is acceptable.

Best regards,
Henson

On Thu, 22 Oct 2026 at 12:57, Henson Choi <assam258@gmail.com> wrote:

CREATE TABLE t (a int, b int);
CREATE VIEW v AS SELECT * FROM t;
CREATE RULE r AS ON UPDATE TO v DO INSTEAD
UPDATE t SET a = new.a, b = new.b WHERE a = old.a
RETURNING WITH (OLD AS y) y.a, t.b;
EXPLAIN (VERBOSE, COSTS OFF)
UPDATE v AS old SET a = old.a + 100
RETURNING WITH (NEW AS y) y.a AS new_a, a, b;

without the Assert()s: y.a, a, t.b
Tom's fragment: y.a, y.a, t.b
your change: y.a, old.a, t.b

I really don't like "y.a, y.a, t.b" here because it looks like it's
outputting the same column twice, and from the point of view of the
person running the query, "y" is NEW, so it appears to output NEW.a
twice.

Additionally, the person running the query may well not be the same
person who wrote the rule, and so they may well not realise that they
inadvertently chose the same alias for NEW as the rule author chose
for OLD.

Printing "y.a, old.a, t.b" isn't perfect either, because "old" is the
alias the query author gave to the view, but the rest of the EXPLAIN
output, makes it clear that the view got replaced with the table "t",
and the alias "old" doesn't appear anywhere else in the EXPLAIN
output, so it's not such a leap to recognise that "old" means OLD, not
the view in the EXPLAIN output.

Regards,
Dean

On Thu, 22 Oct 2026 at 14:39, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

Printing "y.a, old.a, t.b" isn't perfect either, because "old" is the
alias the query author gave to the view

Another option would be to print "y.a, OLD.a, t.b". Putting "OLD" in
capitals, without double-quotes, makes it clear that it's a special
keyword, and it can't clash with a table alias, because a capitalised
table alias would be double-quoted.

Regards,
Dean

Dean Rasheed <dean.a.rasheed@gmail.com> writes:

Another option would be to print "y.a, OLD.a, t.b". Putting "OLD" in
capitals, without double-quotes, makes it clear that it's a special
keyword, and it can't clash with a table alias, because a capitalised
table alias would be double-quoted.

Well, actually not, because EXPLAIN will double-quote it anyway:

Update on public.t
Output: y.a, "OLD".a, t.b

Still, this is probably an improvement on your previous suggestion.

But I think we are putting too much emphasis on this example.
Reality is that if the same alias is specified in both the rule and
the outer query, there is *nothing* we can print that won't be
ambiguous/confusing. That's self-inflicted damage really, and
I'm not very impressed by the argument that two different people
might've written the rule and the query. So I'd rather focus on
what happens when people avoid duplicate aliases, and make sure
that we print names that are as useful as possible in that case.

regards, tom lane