[ returning to this thread now that v19 is open for business ]
Robert Haas <robertmh...@gmail.com> writes:
> I rediscovered, or re-encountered, this problem today, which motivated
> me to have a closer look at your (Tom's) patch. My feeling is that
> it's the right approach. I agree that we could try to keep the current
> generated names by extending addRangeTableEntryForSubquery, but I'm
> tentatively inclined to think that we shouldn't.
That's fine by me. I'm personally content with all of the changes
shown in your patchset.
> However, I did come across one other mildly interesting case.
> expand_single_inheritance_child has this:
> ...
> What I find curious about this is that we're assigning the parent's
> eref to both the child's eref and the child's alias. Maybe there's
> something I don't understand here, or maybe it just doesn't matter,
> but why wouldn't we assign eref to eref and alias to alias? Or even
> alias to alias and generate a new eref?
The issue is explained in the previous comment block a few lines up:
* Construct an alias clause for the child, which we can also use as eref.
* This is important so that EXPLAIN will print the right column aliases
* for child-table columns. (Since ruleutils.c doesn't have any easy way
* to reassociate parent and child columns, we must get the child column
* aliases right to start with. Note that setting childrte->alias forces
* ruleutils.c to use these column names, which it otherwise would not.)
I think the case this is worried about is that if you have a parent
table t(a,b,c), and the query writes say "t as t1(x)", then t's column
"a" will be labeled "x" by EXPLAIN and we want the child's column "a"
to similarly be labeled "x".
So unless we want to start rejiggering ruleutils' already-overly-
complex behavior, we have to put something in childrte->alias, even
if the parent had no alias. So that's a violation of the principle
you were hoping to establish, but as long as it only applies to
partitions and inheritance children, I'm not sure it's worth moving
heaven and earth to make it different.
We could certainly do something a little different than what the code
is doing, such as "if the parent does have a user-written alias, use
parentrte->alias->aliasname not parentrte->eref->aliasname for the
childrte->alias->aliasname". I'm not sure it's worth bothering with
that, though.
I noticed that the patchset was failing in cfbot because we've since
grown another regression test case whose output is affected by 0002.
So here's a v3 that incorporates that additional change. I did a
little bit of wordsmithing on the commit messages too, but the code
changes are identical to v2.
regards, tom lane
From ab46f16a15464e9ba3fae5008ef533e0a6a28303 Mon Sep 17 00:00:00 2001
From: Tom Lane <t...@sss.pgh.pa.us>
Date: Wed, 23 Jul 2025 17:04:45 -0400
Subject: [PATCH v3 1/3] Don't generate fake "*SELECT*" or "*SELECT* %d"
subquery aliases.
rte->alias should point only to a user-written alias, but in these
cases that principle was violated. Fixing this causes some regression
test output changes: wherever rte->alias previously had a value and
is now NULL, rte->eref is now set to a generated name rather than to
rte->alias; and the scheme used to generate eref names differs from
what we were doing for aliases.
The upshot is that instead of "*SELECT*" or "*SELECT* %d",
EXPLAIN will now emit "unnamed_subquery" or "unnamed_subquery_%d".
But that's a reasonable descriptor, and we were already producing
that in yet other cases, so this seems not too objectionable.
Author: Tom Lane <t...@sss.pgh.pa.us>
Co-authored-by: Robert Haas <rh...@postgresql.org>
Discussion: https://postgr.es/m/ca+tgmoysymda2gvanzpmci084n+mvucv0bj0hpbs6uhmmn6...@mail.gmail.com
---
contrib/postgres_fdw/expected/postgres_fdw.out | 8 ++++----
src/backend/executor/functions.c | 2 +-
src/backend/parser/analyze.c | 7 ++-----
src/test/regress/expected/partition_prune.out | 4 ++--
src/test/regress/expected/rangefuncs.out | 8 ++++----
src/test/regress/expected/union.out | 14 +++++++-------
6 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 4b6e49a5d95..c492ba38c58 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -5098,13 +5098,13 @@ SELECT ft1.c1 FROM ft1 JOIN ft2 on ft1.c1 = ft2.c1 WHERE
-- ===================================================================
EXPLAIN (verbose, costs off)
INSERT INTO ft2 (c1,c2,c3) SELECT c1+1000,c2+100, c3 || c3 FROM ft2 LIMIT 20;
- QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Insert on public.ft2
Remote SQL: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
Batch Size: 1
- -> Subquery Scan on "*SELECT*"
- Output: "*SELECT*"."?column?", "*SELECT*"."?column?_1", NULL::integer, "*SELECT*"."?column?_2", NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying(10), 'ft2 '::character(10), NULL::user_enum
+ -> Subquery Scan on unnamed_subquery
+ Output: unnamed_subquery."?column?", unnamed_subquery."?column?_1", NULL::integer, unnamed_subquery."?column?_2", NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying(10), 'ft2 '::character(10), NULL::user_enum
-> Foreign Scan on public.ft2 ft2_1
Output: (ft2_1.c1 + 1000), (ft2_1.c2 + 100), (ft2_1.c3 || ft2_1.c3)
Remote SQL: SELECT "C 1", c2, c3 FROM "S 1"."T 1" LIMIT 20::bigint
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 359aafea681..0547fda2101 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -2454,7 +2454,7 @@ tlist_coercion_finished:
rte = makeNode(RangeTblEntry);
rte->rtekind = RTE_SUBQUERY;
rte->subquery = parse;
- rte->eref = rte->alias = makeAlias("*SELECT*", colnames);
+ rte->eref = makeAlias("unnamed_subquery", colnames);
rte->lateral = false;
rte->inh = false;
rte->inFromCl = true;
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 34f7c17f576..b9763ea1714 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -777,7 +777,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
*/
nsitem = addRangeTableEntryForSubquery(pstate,
selectQuery,
- makeAlias("*SELECT*", NIL),
+ NULL,
false,
false);
addNSItemToQuery(pstate, nsitem, true, false, false);
@@ -2100,7 +2100,6 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt,
{
/* Process leaf SELECT */
Query *selectQuery;
- char selectName[32];
ParseNamespaceItem *nsitem;
RangeTblRef *rtr;
ListCell *tl;
@@ -2156,11 +2155,9 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt,
/*
* Make the leaf query be a subquery in the top-level rangetable.
*/
- snprintf(selectName, sizeof(selectName), "*SELECT* %d",
- list_length(pstate->p_rtable) + 1);
nsitem = addRangeTableEntryForSubquery(pstate,
selectQuery,
- makeAlias(selectName, NIL),
+ NULL,
false,
false);
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index d1966cd7d82..68ecd951809 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -4763,7 +4763,7 @@ select min(a) over (partition by a order by a) from part_abc where a >= stable_o
QUERY PLAN
----------------------------------------------------------------------------------------------
Append
- -> Subquery Scan on "*SELECT* 1_1"
+ -> Subquery Scan on unnamed_subquery_2
-> WindowAgg
Window: w1 AS (PARTITION BY part_abc.a ORDER BY part_abc.a)
-> Append
@@ -4780,7 +4780,7 @@ select min(a) over (partition by a order by a) from part_abc where a >= stable_o
-> Index Scan using part_abc_3_2_a_idx on part_abc_3_2 part_abc_4
Index Cond: (a >= (stable_one() + 1))
Filter: (d <= stable_one())
- -> Subquery Scan on "*SELECT* 2"
+ -> Subquery Scan on unnamed_subquery_1
-> WindowAgg
Window: w1 AS (PARTITION BY part_abc_5.a ORDER BY part_abc_5.a)
-> Append
diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index c21be83aa4a..30241e22da2 100644
--- a/src/test/regress/expected/rangefuncs.out
+++ b/src/test/regress/expected/rangefuncs.out
@@ -2130,10 +2130,10 @@ select testrngfunc();
explain (verbose, costs off)
select * from testrngfunc();
- QUERY PLAN
-----------------------------------------------------------
- Subquery Scan on "*SELECT*"
- Output: "*SELECT*"."?column?", "*SELECT*"."?column?_1"
+ QUERY PLAN
+----------------------------------------------------------------------
+ Subquery Scan on unnamed_subquery
+ Output: unnamed_subquery."?column?", unnamed_subquery."?column?_1"
-> Unique
Output: (1), (2)
-> Sort
diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out
index 96962817ed4..d3ea433db15 100644
--- a/src/test/regress/expected/union.out
+++ b/src/test/regress/expected/union.out
@@ -942,7 +942,7 @@ SELECT q1 FROM int8_tbl EXCEPT SELECT q2 FROM int8_tbl ORDER BY q2 LIMIT 1;
ERROR: column "q2" does not exist
LINE 1: ... int8_tbl EXCEPT SELECT q2 FROM int8_tbl ORDER BY q2 LIMIT 1...
^
-DETAIL: There is a column named "q2" in table "*SELECT* 2", but it cannot be referenced from this part of the query.
+DETAIL: There is a column named "q2" in table "unnamed_subquery", but it cannot be referenced from this part of the query.
-- But this should work:
SELECT q1 FROM int8_tbl EXCEPT (((SELECT q2 FROM int8_tbl ORDER BY q2 LIMIT 1))) ORDER BY 1;
q1
@@ -1338,14 +1338,14 @@ where q2 = q2;
----------------------------------------------------------
Unique
-> Merge Append
- Sort Key: "*SELECT* 1".q1
- -> Subquery Scan on "*SELECT* 1"
+ Sort Key: unnamed_subquery.q1
+ -> Subquery Scan on unnamed_subquery
-> Unique
-> Sort
Sort Key: i81.q1, i81.q2
-> Seq Scan on int8_tbl i81
Filter: (q2 IS NOT NULL)
- -> Subquery Scan on "*SELECT* 2"
+ -> Subquery Scan on unnamed_subquery_1
-> Unique
-> Sort
Sort Key: i82.q1, i82.q2
@@ -1374,14 +1374,14 @@ where -q1 = q2;
--------------------------------------------------------
Unique
-> Merge Append
- Sort Key: "*SELECT* 1".q1
- -> Subquery Scan on "*SELECT* 1"
+ Sort Key: unnamed_subquery.q1
+ -> Subquery Scan on unnamed_subquery
-> Unique
-> Sort
Sort Key: i81.q1, i81.q2
-> Seq Scan on int8_tbl i81
Filter: ((- q1) = q2)
- -> Subquery Scan on "*SELECT* 2"
+ -> Subquery Scan on unnamed_subquery_1
-> Unique
-> Sort
Sort Key: i82.q1, i82.q2
--
2.43.5
From 4bf665212789c0856dd4b166ca9e0359bca3994e Mon Sep 17 00:00:00 2001
From: Robert Haas <rh...@postgresql.org>
Date: Wed, 23 Jul 2025 17:12:34 -0400
Subject: [PATCH v3 2/3] Don't generate fake "ANY_subquery" aliases, either.
This is just like the previous commit, but for a different invented
alias name.
Author: Robert Haas <rh...@postgresql.org>
Reviewed-by: Tom Lane <t...@sss.pgh.pa.us>
Discussion: https://postgr.es/m/ca+tgmoysymda2gvanzpmci084n+mvucv0bj0hpbs6uhmmn6...@mail.gmail.com
---
src/backend/optimizer/plan/subselect.c | 2 +-
src/test/regress/expected/memoize.out | 8 ++++----
src/test/regress/expected/subselect.out | 14 +++++++-------
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index d71ed958e31..fae18548e07 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -1397,7 +1397,7 @@ convert_ANY_sublink_to_join(PlannerInfo *root, SubLink *sublink,
*/
nsitem = addRangeTableEntryForSubquery(pstate,
subselect,
- makeAlias("ANY_subquery", NIL),
+ NULL,
use_lateral,
false);
rte = nsitem->p_rte;
diff --git a/src/test/regress/expected/memoize.out b/src/test/regress/expected/memoize.out
index 150dc1b44cf..fbcaf113266 100644
--- a/src/test/regress/expected/memoize.out
+++ b/src/test/regress/expected/memoize.out
@@ -545,15 +545,15 @@ EXPLAIN (COSTS OFF)
SELECT * FROM tab_anti t1 WHERE t1.a IN
(SELECT a FROM tab_anti t2 WHERE t2.b IN
(SELECT t1.b FROM tab_anti t3 WHERE t2.a > 1 OFFSET 0));
- QUERY PLAN
--------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------
Nested Loop Semi Join
-> Seq Scan on tab_anti t1
-> Nested Loop Semi Join
Join Filter: (t1.a = t2.a)
-> Seq Scan on tab_anti t2
- -> Subquery Scan on "ANY_subquery"
- Filter: (t2.b = "ANY_subquery".b)
+ -> Subquery Scan on unnamed_subquery
+ Filter: (t2.b = unnamed_subquery.b)
-> Result
One-Time Filter: (t2.a > 1)
-> Seq Scan on tab_anti t3
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index 18fed63e738..54df8fcbba9 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -1467,14 +1467,14 @@ select * from int4_tbl o where (f1, f1) in
-------------------------------------------------------------------
Nested Loop Semi Join
Output: o.f1
- Join Filter: (o.f1 = "ANY_subquery".f1)
+ Join Filter: (o.f1 = unnamed_subquery.f1)
-> Seq Scan on public.int4_tbl o
Output: o.f1
-> Materialize
- Output: "ANY_subquery".f1, "ANY_subquery".g
- -> Subquery Scan on "ANY_subquery"
- Output: "ANY_subquery".f1, "ANY_subquery".g
- Filter: ("ANY_subquery".f1 = "ANY_subquery".g)
+ Output: unnamed_subquery.f1, unnamed_subquery.g
+ -> Subquery Scan on unnamed_subquery
+ Output: unnamed_subquery.f1, unnamed_subquery.g
+ Filter: (unnamed_subquery.f1 = unnamed_subquery.g)
-> Result
Output: i.f1, ((generate_series(1, 50)) / 10)
-> ProjectSet
@@ -2642,8 +2642,8 @@ ON B.hundred in (SELECT min(c.hundred) FROM tenk2 C WHERE c.odd = b.odd);
-> Memoize
Cache Key: b.hundred, b.odd
Cache Mode: binary
- -> Subquery Scan on "ANY_subquery"
- Filter: (b.hundred = "ANY_subquery".min)
+ -> Subquery Scan on unnamed_subquery
+ Filter: (b.hundred = unnamed_subquery.min)
-> Result
InitPlan 1
-> Limit
--
2.43.5
From e92da9d1520e8feb71aa6fd42515c4e34b862535 Mon Sep 17 00:00:00 2001
From: Robert Haas <rh...@postgresql.org>
Date: Wed, 23 Jul 2025 17:16:20 -0400
Subject: [PATCH v3 3/3] Don't generate fake "*TLOCRN*" or "*TROCRN*" aliases,
either.
This fix actually doesn't change any regression test outputs.
Author: Robert Haas <rh...@postgresql.org>
Reviewed-by: Tom Lane <t...@sss.pgh.pa.us>
Discussion: https://postgr.es/m/ca+tgmoysymda2gvanzpmci084n+mvucv0bj0hpbs6uhmmn6...@mail.gmail.com
---
src/backend/rewrite/rewriteSearchCycle.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/rewrite/rewriteSearchCycle.c b/src/backend/rewrite/rewriteSearchCycle.c
index 19b89dee0d0..3b1a9dfc576 100644
--- a/src/backend/rewrite/rewriteSearchCycle.c
+++ b/src/backend/rewrite/rewriteSearchCycle.c
@@ -282,8 +282,8 @@ rewriteSearchAndCycle(CommonTableExpr *cte)
newrte = makeNode(RangeTblEntry);
newrte->rtekind = RTE_SUBQUERY;
- newrte->alias = makeAlias("*TLOCRN*", cte->ctecolnames);
- newrte->eref = newrte->alias;
+ newrte->alias = NULL;
+ newrte->eref = makeAlias("*TLOCRN*", cte->ctecolnames);
newsubquery = copyObject(rte1->subquery);
IncrementVarSublevelsUp((Node *) newsubquery, 1, 1);
newrte->subquery = newsubquery;
@@ -379,8 +379,8 @@ rewriteSearchAndCycle(CommonTableExpr *cte)
ewcl = lappend(ewcl, makeString(cte->cycle_clause->cycle_mark_column));
ewcl = lappend(ewcl, makeString(cte->cycle_clause->cycle_path_column));
}
- newrte->alias = makeAlias("*TROCRN*", ewcl);
- newrte->eref = newrte->alias;
+ newrte->alias = NULL;
+ newrte->eref = makeAlias("*TROCRN*", ewcl);
/*
* Find the reference to the recursive CTE in the right UNION subquery's
--
2.43.5
