From 6b858a36ddb80bfa4f478afebe3e84de72600e02 Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Tue, 6 Oct 2026 10:56:35 +0900 Subject: [PATCH] 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 Reviewed-by: Richard Guo 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)