database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-10-23 13:29:09 +03:00
Code Activity

Fix stale copies of PHVs in subqueries

Browse Source 
When a subquery references an output of another subquery that gets
pulled up, and that output must be wrapped in a PlaceHolderVar because
of an intermediate outer join, the PHV expression is pushed down into
the subquery.  That copy is not preprocessed along with the outer
query's expressions, so the two copies can diverge.  This used to be
harmless, but since commit 2ebf25e7d join removal edits the whole
query tree, walking into subqueries, and can trip an assert in
ChangeVarNodes if it removes a rel whose Var survives only in such a
copy.

To fix, preprocess these copies at their owning query level, early in
subquery_planner, before anything can consume them (in particular
before SubLinks are turned into SubPlans).  This covers copies pushed
into both LATERAL subquery RTEs and SubLink subselects, and handles
nested copies innermost-first.  extract_lateral_references no longer
preprocesses the copies it pulls out.

Correspondingly, the subquery's own processing must leave the contents
of an upper-level PHV alone, since the owning level has already
preprocessed them: eval_const_expressions returns such a PHV
unchanged, and flatten_join_alias_vars no longer recurses into it.

Back-patch to v16, as with commit 2ebf25e7d.

Reported-by: Tender Wang <tndrwang@gmail.com>
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAHewXN=kWGAXV537mKtSyBYobGdHhYJVDJJMXXZEmmPWE_zaPw@mail.gmail.com
Backpatch-through: 16
This commit is contained in:
Richard Guo
2026-09-14 12:29:44 +09:00
parent 25d6716cef
commit ccb7d9dba1
8 changed files with 344 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/backend/optimizer/plan/initsplan.c
View File

@@ -26,7 +26,6 @@
#include "optimizer/paths.h"
#include "optimizer/placeholder.h"
#include "optimizer/planmain.h"
#include "optimizer/planner.h"
#include "optimizer/restrictinfo.h"
#include "parser/analyze.h"
#include "rewrite/rewriteManip.h"
@@ -453,17 +452,13 @@ extract_lateral_references(PlannerInfo *root, RelOptInfo *brel, Index rtindex)
PlaceHolderVar *phv = (PlaceHolderVar *) node;
int levelsup = phv->phlevelsup;
/* Have to work harder to adjust the contained expression too */
/*
* Have to work harder to adjust the contained expression too.
* (Its expression has already been preprocessed by
* subquery_planner(), so we must not do that again here.)
*/
if (levelsup != 0)
IncrementVarSublevelsUp(node, -levelsup, 0);
/*
* If we pulled the PHV out of a subquery RTE, its expression
* needs to be preprocessed. subquery_planner() already did this
* for level-zero PHVs in function and values RTEs, though.
*/
if (levelsup > 0)
phv->phexpr = preprocess_phv_expression(root, phv->phexpr);
}
else
Assert(false);

90
src/backend/optimizer/plan/planner.c
View File

@@ -58,6 +58,7 @@
#include "parser/parse_relation.h"
#include "parser/parsetree.h"
#include "partitioning/partdesc.h"
#include "rewrite/rewriteManip.h"
#include "utils/acl.h"
#include "utils/lsyscache.h"
#include "utils/rel.h"
@@ -125,8 +126,18 @@ typedef struct
* subquery belonging to a set operation */
} standard_qp_extra;
/* Context for preprocess_subquery_phvs_walker */
typedef struct
{
PlannerInfo *root;
int sublevels_up;
} preprocess_subquery_phvs_context;
/* Local functions */
static Node *preprocess_expression(PlannerInfo *root, Node *expr, int kind);
static void preprocess_subquery_phvs(PlannerInfo *root);
static bool preprocess_subquery_phvs_walker(Node *node,
preprocess_subquery_phvs_context *context);
static void preprocess_qual_conditions(PlannerInfo *root, Node *jtnode);
static void grouping_planner(PlannerInfo *root, double tuple_fraction,
SetOperationStmt *setops);
@@ -845,6 +856,14 @@ subquery_planner(PlannerGlobal *glob, Query *parse, PlannerInfo *parent_root,
*/
root->hasHavingQual = (parse->havingQual != NULL);
/*
* Preprocess any PlaceHolderVars of our level that were pushed down into
* subqueries. This must happen before anything can consume those copies,
* in particular before SubLinks below are turned into SubPlans.
*/
if (root->glob->lastPHId != 0)
preprocess_subquery_phvs(root);
/*
* Do expression preprocessing on targetlist and quals, as well as other
* random expressions in the querytree. Note that we do not need to
@@ -1290,6 +1309,77 @@ preprocess_phv_expression(PlannerInfo *root, Expr *expr)
return (Expr *) preprocess_expression(root, (Node *) expr, EXPRKIND_PHV);
}
/*
* preprocess_subquery_phvs
* Preprocess copies of this level's PlaceHolderVars that were pushed
* down into subqueries.
*
* When a subquery (a LATERAL RTE or a SubLink's subselect) references a
* pulled-up output that must be wrapped in a PlaceHolderVar, the PHV
* expression is pushed down into the subquery. The subquery's own planning
* leaves that copy alone, since it belongs to our level, so we need to
* preprocess it. We modify the PHVs in place, temporarily adjusting each to
* our level, and handle nested copies innermost-first.
*/
static void
preprocess_subquery_phvs(PlannerInfo *root)
{
preprocess_subquery_phvs_context context;
context.root = root;
context.sublevels_up = 0;
(void) query_tree_walker(root->parse, preprocess_subquery_phvs_walker,
&context, 0);
}
static bool
preprocess_subquery_phvs_walker(Node *node,
preprocess_subquery_phvs_context *context)
{
if (node == NULL)
return false;
if (IsA(node, Query))
{
bool result;
context->sublevels_up++;
result = query_tree_walker((Query *) node,
preprocess_subquery_phvs_walker,
context, 0);
context->sublevels_up--;
return result;
}
if (IsA(node, PlaceHolderVar))
{
PlaceHolderVar *phv = (PlaceHolderVar *) node;
/* Handle any nested copies within the expression first */
(void) expression_tree_walker(node, preprocess_subquery_phvs_walker,
context);
/*
* Is this a copy of one of our PHVs that is pushed down into a
* subquery?
*/
if (context->sublevels_up > 0 &&
phv->phlevelsup == context->sublevels_up)
{
int levelsup = phv->phlevelsup;
Node *expr;
/* Adjust the expression to our level, preprocess, adjust back */
expr = copyObject((Node *) phv->phexpr);
IncrementVarSublevelsUp(expr, -levelsup, 0);
expr = preprocess_expression(context->root, expr, EXPRKIND_PHV);
IncrementVarSublevelsUp(expr, levelsup, 0);
phv->phexpr = (Expr *) expr;
}
return false;
}
return expression_tree_walker(node, preprocess_subquery_phvs_walker,
context);
}
/*--------------------
* grouping_planner
* Perform planning steps related to grouping, aggregation, etc.

12
src/backend/optimizer/plan/subselect.c
View File

@@ -358,12 +358,14 @@ build_subplan(PlannerInfo *root, Plan *plan, Path *path,
* adjusted to have the correct varlevelsup, phlevelsup, or
* agglevelsup.
*
* If it's a PlaceHolderVar, Aggref or GroupingFunc, its arguments
* might contain SubLinks, which have not yet been processed (see the
* comments for SS_replace_correlation_vars). Do that now.
* If it's an Aggref or GroupingFunc, its arguments might contain
* SubLinks, which have not yet been processed (see the 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.
*/
if (IsA(arg, PlaceHolderVar) ||
IsA(arg, Aggref) ||
if (IsA(arg, Aggref) ||
IsA(arg, GroupingFunc))
arg = SS_process_sublinks(root, arg, false);

35
src/backend/optimizer/util/clauses.c
View File

@@ -2952,9 +2952,8 @@ eval_const_expressions_mutator(Node *node,
/*
* Return a SubPlan unchanged --- too late to do anything with it.
*
* XXX should we ereport() here instead? Probably this routine
* should never be invoked after SubPlan creation.
* This can happen in estimation mode, which runs after SubPlans
* have been created.
*/
return node;
case T_RelabelType:
@@ -3658,20 +3657,28 @@ eval_const_expressions_mutator(Node *node,
return (Node *) newcdomain;
}
case T_PlaceHolderVar:
/*
* In estimation mode, just strip the PlaceHolderVar node
* altogether; this amounts to estimating that the contained value
* won't be forced to null by an outer join. In regular mode we
* just use the default behavior (ie, simplify the expression but
* leave the PlaceHolderVar node intact).
*/
if (context->estimate)
{
PlaceHolderVar *phv = (PlaceHolderVar *) node;
return eval_const_expressions_mutator((Node *) phv->phexpr,
context);
/*
* Leave a PHV of an upper query level alone: its expression
* belongs to that level, which has already preprocessed it.
* But we do copy the subtree, just to conform to this
* function's API spec.
*/
if (phv->phlevelsup > 0)
return copyObject(node);
/*
* In estimation mode, just strip the PlaceHolderVar node
* altogether; this amounts to estimating that the contained
* value won't be forced to null by an outer join. In regular
* mode we just use the default behavior (ie, simplify the
* expression but leave the PlaceHolderVar node intact).
*/
if (context->estimate)
return eval_const_expressions_mutator((Node *) phv->phexpr,
context);
}
break;
case T_ConvertRowtypeExpr:

33
src/backend/optimizer/util/var.c
View File

@@ -857,12 +857,35 @@ flatten_join_alias_vars_mutator(Node *node,
}
if (IsA(node, PlaceHolderVar))
{
/* Copy the PlaceHolderVar node with correct mutation of subnodes */
PlaceHolderVar *phv;
PlaceHolderVar *phv = (PlaceHolderVar *) node;
phv = (PlaceHolderVar *) expression_tree_mutator(node,
flatten_join_alias_vars_mutator,
(void *) context);
/*
* A PHV above the target level can't contain join aliases of the
* target level and has no relid sets of ours to fix, so return it
* unchanged.
*/
if (phv->phlevelsup > context->sublevels_up)
return node; /* no need to copy, really */
/*
* A pushed-down copy of a target-level PHV was already preprocessed
* by preprocess_subquery_phvs, so it may contain SubPlans and holds
* no unflattened target-level aliases; shallow-copy it instead of
* recursing. Any other PHV is recursed into, as it may still hold
* target-level aliases.
*/
if (phv->phlevelsup == context->sublevels_up &&
context->sublevels_up > 0)
{
PlaceHolderVar *newphv = makeNode(PlaceHolderVar);
memcpy(newphv, phv, sizeof(PlaceHolderVar));
phv = newphv;
}
else
phv = (PlaceHolderVar *) expression_tree_mutator(node,
flatten_join_alias_vars_mutator,
(void *) context);
/* now fix PlaceHolderVar's relid sets */
if (phv->phlevelsup == context->sublevels_up)
{

145
src/test/regress/expected/join.out
View File

@@ -5780,6 +5780,151 @@ on lhs.id = rhs.id;
-> Result
(5 rows)
-- check handling of a removed Var that's pushed down into a subquery
-- (fallout from the fix for bug #19560)
explain (verbose, costs off)
select c1, c2
from (select case when false then remov.id end as c1
from int4_tbl i41 left join a remov on i41.f1 = remov.id) ss1
right join int4_tbl i42 on false,
lateral (select ss1.c1 as c2 from int4_tbl i43 offset 0) ss2;
QUERY PLAN
----------------------------------------------
Nested Loop
Output: (NULL::integer), ((NULL::integer))
-> Nested Loop Left Join
Output: (NULL::integer)
Join Filter: false
-> Seq Scan on public.int4_tbl i42
Output: i42.f1
-> Result
Output: NULL::integer
One-Time Filter: false
-> Memoize
Output: ((NULL::integer))
Cache Key: (NULL::integer)
Cache Mode: binary
-> Seq Scan on public.int4_tbl i43
Output: (NULL::integer)
(16 rows)
-- likewise, where the subquery is a UNION ALL whose arms are appendrel
-- children that are not simple enough to be pulled up
explain (verbose, costs off)
select c1, c2
from (select case when false then remov.id end as c1
from int4_tbl i41 left join a remov on i41.f1 = remov.id) ss1
right join int4_tbl i42 on false,
lateral ((select ss1.c1 as c2 from int4_tbl i43 offset 0)
union all
(select ss1.c1 from int4_tbl i44 offset 0)) ss2;
QUERY PLAN
---------------------------------------------------
Nested Loop
Output: (NULL::integer), ((NULL::integer))
-> Nested Loop Left Join
Output: (NULL::integer)
Join Filter: false
-> Seq Scan on public.int4_tbl i42
Output: i42.f1
-> Result
Output: NULL::integer
One-Time Filter: false
-> Memoize
Output: ((NULL::integer))
Cache Key: (NULL::integer)
Cache Mode: binary
-> Append
-> Seq Scan on public.int4_tbl i43
Output: (NULL::integer)
-> Seq Scan on public.int4_tbl i44
Output: (NULL::integer)
(19 rows)
-- likewise, where the PHV contains a SubPlan and the subquery has a join, so
-- that its planner runs flatten_join_alias_vars over the outer-level PHV
explain (verbose, costs off)
select c1, c2
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 false,
lateral (select ss1.c1 as c2 from int4_tbl i43 join int4_tbl i44 on true
offset 0) ss2;
QUERY PLAN
---------------------------------------------------------
Nested Loop
Output: ((SubPlan 2)), (((SubPlan 2)))
-> Nested Loop Left Join
Output: ((SubPlan 2))
Join Filter: false
-> Seq Scan on public.int4_tbl i42
Output: i42.f1
-> Result
Output: (SubPlan 2)
One-Time Filter: false
SubPlan 2
-> Result
Output: f1
-> Memoize
Output: (((SubPlan 2)))
Cache Key: ((SubPlan 2))
Cache Mode: binary
-> Nested Loop
Output: ((SubPlan 2))
-> Seq Scan on public.int4_tbl i43
Output: i43.f1
-> Materialize
-> Seq Scan on public.int4_tbl i44
(23 rows)
-- likewise, where the PHV copy is pushed into a SubLink's subselect rather
-- than a LATERAL subquery
explain (verbose, costs off)
select (select ss1.c1 from int4_tbl i43 offset 0) as c2
from (select case when false then remov.id end as c1
from int4_tbl i41 left join a remov on i41.f1 = remov.id) ss1
right join int4_tbl i42 on true;
QUERY PLAN
---------------------------------------------
Nested Loop Left Join
Output: (SubPlan 1)
-> Seq Scan on public.int4_tbl i42
Output: i42.f1
-> Materialize
Output: (NULL::integer)
-> Seq Scan on public.int4_tbl i41
Output: NULL::integer
SubPlan 1
-> Seq Scan on public.int4_tbl i43
Output: (NULL::integer)
(11 rows)
-- likewise, where the pushed-down PHV's expression itself contains a SubLink,
-- so it must be preprocessed once at the outer level rather than again while
-- building the SubPlan that references it
explain (verbose, costs off)
select (select ss1.c1 from int4_tbl i43 offset 0) as c2
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;
QUERY PLAN
---------------------------------------------
Nested Loop Left Join
Output: (SubPlan 2)
-> Seq Scan on public.int4_tbl i42
Output: i42.f1
-> Materialize
Output: ((SubPlan 1))
-> Seq Scan on public.int4_tbl i41
Output: (SubPlan 1)
SubPlan 1
-> Result
Output: i41.f1
SubPlan 2
-> Seq Scan on public.int4_tbl i43
Output: ((SubPlan 1))
(14 rows)
-- More tests of correct placement of pseudoconstant quals
-- simple constant-false condition
explain (costs off)

47
src/test/regress/sql/join.sql
View File

@@ -2117,6 +2117,53 @@ full join
) as rhs
on lhs.id = rhs.id;
-- check handling of a removed Var that's pushed down into a subquery
-- (fallout from the fix for bug #19560)
explain (verbose, costs off)
select c1, c2
from (select case when false then remov.id end as c1
from int4_tbl i41 left join a remov on i41.f1 = remov.id) ss1
right join int4_tbl i42 on false,
lateral (select ss1.c1 as c2 from int4_tbl i43 offset 0) ss2;
-- likewise, where the subquery is a UNION ALL whose arms are appendrel
-- children that are not simple enough to be pulled up
explain (verbose, costs off)
select c1, c2
from (select case when false then remov.id end as c1
from int4_tbl i41 left join a remov on i41.f1 = remov.id) ss1
right join int4_tbl i42 on false,
lateral ((select ss1.c1 as c2 from int4_tbl i43 offset 0)
union all
(select ss1.c1 from int4_tbl i44 offset 0)) ss2;
-- likewise, where the PHV contains a SubPlan and the subquery has a join, so
-- that its planner runs flatten_join_alias_vars over the outer-level PHV
explain (verbose, costs off)
select c1, c2
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 false,
lateral (select ss1.c1 as c2 from int4_tbl i43 join int4_tbl i44 on true
offset 0) ss2;
-- likewise, where the PHV copy is pushed into a SubLink's subselect rather
-- than a LATERAL subquery
explain (verbose, costs off)
select (select ss1.c1 from int4_tbl i43 offset 0) as c2
from (select case when false then remov.id 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's expression itself contains a SubLink,
-- so it must be preprocessed once at the outer level rather than again while
-- building the SubPlan that references it
explain (verbose, costs off)
select (select ss1.c1 from int4_tbl i43 offset 0) as c2
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;
-- More tests of correct placement of pseudoconstant quals
-- simple constant-false condition

1
src/tools/pgindent/typedefs.list
View File

@@ -3782,6 +3782,7 @@ post_parse_analyze_hook_type
postprocess_result_function
pqbool
pqsigfunc
preprocess_subquery_phvs_context
printQueryOpt
printTableContent
printTableFooter