postgres.git / summary / log / commit / refs
commit 6b858a36ddb80bfa4f478afebe3e84de72600e02
Author: Richard Guo <rguo@postgresql.org>
Date: Tue Oct 06 01:56:35 2026 +0000
Don't reprocess PHV copies within outer aggregates' arguments
Since commit e28cf96e9, the copies of a query level's PlaceHolderVars
that were pushed down into its subqueries are preprocessed by that
level before its SubLinks are turned into SubPlans, so such a copy may
already contain SubPlans by the time the subquery is planned. That
commit taught build_subplan not to process the SubLinks of a PHV that
is passed down as a SubPlan argument, but overlooked that such a copy
can also appear within the arguments of an outer-level Aggref,
GroupingFunc, or ReturningExpr. The SubLinks in those arguments are
still processed when build_subplan collects the node, and that
processing descended into the already-preprocessed PHV and tripped the
no-SubPlan assertion in process_sublinks_mutator.
To fix, don't descend into PHVs when processing the SubLinks in such a
node's arguments. Any PHV found there is a pushed-down copy that its
owning level has already preprocessed.
Back-patch to v16, as with commit e28cf96e9.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAJTYsWW8THbrnsmYnDqnUgg0PjGrqw2vNOWb8qx7eu191KPW3g@mail.gmail.com
Backpatch-through: 16
src/backend/optimizer/plan/subselect.c | 18 +++++++++++++++---
src/test/regress/expected/join.out | 25 +++++++++++++++++++++++++
src/test/regress/sql/join.sql | 6 ++++++
3 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 5760b616813..7df1a1cb514 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -50,6 +50,7 @@ typedef struct process_sublinks_context
{
PlannerInfo *root;
bool isTopQual;
+ bool skipPHVs; /* don't descend into PlaceHolderVars? */
} process_sublinks_context;
typedef struct finalize_primnode_context
@@ -376,12 +377,20 @@ build_subplan(PlannerInfo *root, Plan *plan, Path *path,
* comments for SS_replace_correlation_vars). Do that now. A
* PlaceHolderVar needs no such treatment: subquery_planner already
* preprocessed the PHVs of its owning level, so its expression is
- * fully processed and may already contain SubPlans.
+ * fully processed and may already contain SubPlans. The same goes
+ * for any PlaceHolderVars within the arguments, so skip those.
*/
if (IsA(arg, Aggref) ||
IsA(arg, GroupingFunc) ||
IsA(arg, ReturningExpr))
- arg = SS_process_sublinks(root, arg, false);
+ {
+ process_sublinks_context context;
+
+ context.root = root;
+ context.isTopQual = false;
+ context.skipPHVs = true;
+ arg = process_sublinks_mutator(arg, &context);
+ }
splan->parParam = lappend_int(splan->parParam, pitem->paramId);
splan->args = lappend(splan->args, arg);
@@ -2217,6 +2226,7 @@ SS_process_sublinks(PlannerInfo *root, Node *expr, bool isQual)
context.root = root;
context.isTopQual = isQual;
+ context.skipPHVs = false;
return process_sublinks_mutator(expr, &context);
}
@@ -2226,6 +2236,7 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context)
process_sublinks_context locContext;
locContext.root = context->root;
+ locContext.skipPHVs = context->skipPHVs;
if (node == NULL)
return NULL;
@@ -2259,10 +2270,11 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context)
* ReturningExpr they'll be handled when build_subplan collects it into
* the arguments to be passed down to the current subplan, while an outer
* PHV's expression has already been preprocessed by its owning level.
+ * That also holds for a PHV within such an argument (skipPHVs).
*/
if (IsA(node, PlaceHolderVar))
{
- if (((PlaceHolderVar *) node)->phlevelsup > 0)
+ if (((PlaceHolderVar *) node)->phlevelsup > 0 || context->skipPHVs)
return node;
}
else if (IsA(node, Aggref))
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index f8a4b14da49..88d9d362d7c 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -7430,6 +7430,31 @@ from (select case when false then remov.id else (select i41.f1) end as c1
Output: ((SubPlan expr_1))
(14 rows)
+-- likewise, where the pushed-down PHV sits within an outer-level aggregate
+explain (verbose, costs off)
+select (select sum(ss1.c1) from int4_tbl i43) as c2
+from (select (select i41.f1) as c1 from int4_tbl i41) ss1
+ right join int4_tbl i42 on true;
+ QUERY PLAN
+---------------------------------------------------
+ Aggregate
+ Output: (SubPlan expr_2)
+ -> Nested Loop Left Join
+ Output: ((SubPlan expr_1))
+ -> Seq Scan on public.int4_tbl i42
+ Output: i42.f1
+ -> Materialize
+ Output: ((SubPlan expr_1))
+ -> Seq Scan on public.int4_tbl i41
+ Output: (SubPlan expr_1)
+ SubPlan expr_1
+ -> Result
+ Output: i41.f1
+ SubPlan expr_2
+ -> Seq Scan on public.int4_tbl i43
+ Output: sum(((SubPlan expr_1)))
+(16 rows)
+
-- likewise, where the pushed-down PHV's expression contains another PHV of
-- the same level, which must not be preprocessed separately from its parent
explain (verbose, costs off)
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 7c5346fa462..f6cf18b8872 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -2746,6 +2746,12 @@ from (select case when false then remov.id else (select i41.f1) end as c1
from int4_tbl i41 left join a remov on i41.f1 = remov.id) ss1
right join int4_tbl i42 on true;
+-- likewise, where the pushed-down PHV sits within an outer-level aggregate
+explain (verbose, costs off)
+select (select sum(ss1.c1) from int4_tbl i43) as c2
+from (select (select i41.f1) as c1 from int4_tbl i41) ss1
+ right join int4_tbl i42 on true;
+
-- likewise, where the pushed-down PHV's expression contains another PHV of
-- the same level, which must not be preprocessed separately from its parent
explain (verbose, costs off)
[parent: 80707c10456f]