Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitcb8e50a

Browse files
committed
Avoid inserting PlaceHolderVars in cases where pre-v16 PG did not.
Commit2489d76 removed some logic from pullup_replace_vars()that avoided wrapping a PlaceHolderVar around a pulled-upsubquery output expression if the expression could be provento go to NULL anyway (because it contained Vars or PHVs of thepulled-up relation and did not contain non-strict constructs).But removing that logic turns out to cause performance regressionsin some cases, because the extra PHV blocks subexpression folding,and will do so even if outer-join reduction later turns it into ano-op with no phnullingrels bits. This can for example preventan expression from being matched to an index.The reason for always adding a PHV was to ensure we had someplaceto put the varnullingrels marker bits of the Var being replaced.However, it turns out we can optimize in exactly the same cases thatthe previous code did, because we can instead attach the neededvarnullingrels bits to the contained Var(s)/PHV(s).This is not a complete solution --- it would be even better if wecould remove PHVs after reducing them to no-ops. It doesn't lookpractical to back-patch such an improvement, but this change seemssafe and at least gets rid of the performance-regression cases.Per complaint from Nikhil Raj. Back-patch to v16 where theproblem appeared.Discussion:https://postgr.es/m/CAG1ps1xvnTZceKK24OUfMKLPvDP2vjT-d+F2AOCWbw_v3KeEgg@mail.gmail.com
1 parent3409b4d commitcb8e50a

File tree

4 files changed

+107
-15
lines changed

4 files changed

+107
-15
lines changed

‎src/backend/optimizer/prep/prepjointree.c

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2490,14 +2490,48 @@ pullup_replace_vars_callback(Var *var,
24902490
else
24912491
wrap= false;
24922492
}
2493+
elseif (rcon->wrap_non_vars)
2494+
{
2495+
/* Caller told us to wrap all non-Vars in a PlaceHolderVar */
2496+
wrap= true;
2497+
}
24932498
else
24942499
{
24952500
/*
2496-
* Must wrap, either because we need a place to insert
2497-
* varnullingrels or because caller told us to wrap
2498-
* everything.
2501+
* If the node contains Var(s) or PlaceHolderVar(s) of the
2502+
* subquery being pulled up, and does not contain any
2503+
* non-strict constructs, then instead of adding a PHV on top
2504+
* we can add the required nullingrels to those Vars/PHVs.
2505+
* (This is fundamentally a generalization of the above cases
2506+
* for bare Vars and PHVs.)
2507+
*
2508+
* This test is somewhat expensive, but it avoids pessimizing
2509+
* the plan in cases where the nullingrels get removed again
2510+
* later by outer join reduction.
2511+
*
2512+
* This analysis could be tighter: in particular, a non-strict
2513+
* construct hidden within a lower-level PlaceHolderVar is not
2514+
* reason to add another PHV. But for now it doesn't seem
2515+
* worth the code to be more exact.
2516+
*
2517+
* For a LATERAL subquery, we have to check the actual var
2518+
* membership of the node, but if it's non-lateral then any
2519+
* level-zero var must belong to the subquery.
24992520
*/
2500-
wrap= true;
2521+
if ((rcon->target_rte->lateral ?
2522+
bms_overlap(pull_varnos(rcon->root,newnode),
2523+
rcon->relids) :
2524+
contain_vars_of_level(newnode,0))&&
2525+
!contain_nonstrict_functions(newnode))
2526+
{
2527+
/* No wrap needed */
2528+
wrap= false;
2529+
}
2530+
else
2531+
{
2532+
/* Else wrap it in a PlaceHolderVar */
2533+
wrap= true;
2534+
}
25012535
}
25022536

25032537
if (wrap)
@@ -2518,33 +2552,41 @@ pullup_replace_vars_callback(Var *var,
25182552
}
25192553
}
25202554

2521-
/* Must adjust varlevelsup if replaced Var is within a subquery */
2522-
if (var->varlevelsup>0)
2523-
IncrementVarSublevelsUp(newnode,var->varlevelsup,0);
2524-
2525-
/* Propagate any varnullingrels into the replacement Var or PHV */
2555+
/* Propagate any varnullingrels into the replacement expression */
25262556
if (var->varnullingrels!=NULL)
25272557
{
25282558
if (IsA(newnode,Var))
25292559
{
25302560
Var*newvar= (Var*)newnode;
25312561

2532-
Assert(newvar->varlevelsup==var->varlevelsup);
2562+
Assert(newvar->varlevelsup==0);
25332563
newvar->varnullingrels=bms_add_members(newvar->varnullingrels,
25342564
var->varnullingrels);
25352565
}
25362566
elseif (IsA(newnode,PlaceHolderVar))
25372567
{
25382568
PlaceHolderVar*newphv= (PlaceHolderVar*)newnode;
25392569

2540-
Assert(newphv->phlevelsup==var->varlevelsup);
2570+
Assert(newphv->phlevelsup==0);
25412571
newphv->phnullingrels=bms_add_members(newphv->phnullingrels,
25422572
var->varnullingrels);
25432573
}
25442574
else
2545-
elog(ERROR,"failed to wrap a non-Var");
2575+
{
2576+
/* There should be lower-level Vars/PHVs we can modify */
2577+
newnode=add_nulling_relids(newnode,
2578+
NULL,/* modify all Vars/PHVs */
2579+
var->varnullingrels);
2580+
/* Assert we did put the varnullingrels into the expression */
2581+
Assert(bms_is_subset(var->varnullingrels,
2582+
pull_varnos(rcon->root,newnode)));
2583+
}
25462584
}
25472585

2586+
/* Must adjust varlevelsup if replaced Var is within a subquery */
2587+
if (var->varlevelsup>0)
2588+
IncrementVarSublevelsUp(newnode,var->varlevelsup,0);
2589+
25482590
returnnewnode;
25492591
}
25502592

‎src/backend/rewrite/rewriteManip.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,7 +1141,8 @@ AddInvertedQual(Query *parsetree, Node *qual)
11411141
/*
11421142
* add_nulling_relids() finds Vars and PlaceHolderVars that belong to any
11431143
* of the target_relids, and adds added_relids to their varnullingrels
1144-
* and phnullingrels fields.
1144+
* and phnullingrels fields. If target_relids is NULL, all level-zero
1145+
* Vars and PHVs are modified.
11451146
*/
11461147
Node*
11471148
add_nulling_relids(Node*node,
@@ -1170,7 +1171,8 @@ add_nulling_relids_mutator(Node *node,
11701171
Var*var= (Var*)node;
11711172

11721173
if (var->varlevelsup==context->sublevels_up&&
1173-
bms_is_member(var->varno,context->target_relids))
1174+
(context->target_relids==NULL||
1175+
bms_is_member(var->varno,context->target_relids)))
11741176
{
11751177
Relidsnewnullingrels=bms_union(var->varnullingrels,
11761178
context->added_relids);
@@ -1188,7 +1190,8 @@ add_nulling_relids_mutator(Node *node,
11881190
PlaceHolderVar*phv= (PlaceHolderVar*)node;
11891191

11901192
if (phv->phlevelsup==context->sublevels_up&&
1191-
bms_overlap(phv->phrels,context->target_relids))
1193+
(context->target_relids==NULL||
1194+
bms_overlap(phv->phrels,context->target_relids)))
11921195
{
11931196
Relidsnewnullingrels=bms_union(phv->phnullingrels,
11941197
context->added_relids);

‎src/test/regress/expected/subselect.out

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1721,6 +1721,35 @@ fetch backward all in c1;
17211721
(2 rows)
17221722

17231723
commit;
1724+
--
1725+
-- Verify that we correctly flatten cases involving a subquery output
1726+
-- expression that doesn't need to be wrapped in a PlaceHolderVar
1727+
--
1728+
explain (costs off)
1729+
select tname, attname from (
1730+
select relname::information_schema.sql_identifier as tname, * from
1731+
(select * from pg_class c) ss1) ss2
1732+
right join pg_attribute a on a.attrelid = ss2.oid
1733+
where tname = 'tenk1' and attnum = 1;
1734+
QUERY PLAN
1735+
--------------------------------------------------------------------------
1736+
Nested Loop
1737+
-> Index Scan using pg_class_relname_nsp_index on pg_class c
1738+
Index Cond: (relname = 'tenk1'::name)
1739+
-> Index Scan using pg_attribute_relid_attnum_index on pg_attribute a
1740+
Index Cond: ((attrelid = c.oid) AND (attnum = 1))
1741+
(5 rows)
1742+
1743+
select tname, attname from (
1744+
select relname::information_schema.sql_identifier as tname, * from
1745+
(select * from pg_class c) ss1) ss2
1746+
right join pg_attribute a on a.attrelid = ss2.oid
1747+
where tname = 'tenk1' and attnum = 1;
1748+
tname | attname
1749+
-------+---------
1750+
tenk1 | unique1
1751+
(1 row)
1752+
17241753
--
17251754
-- Tests for CTE inlining behavior
17261755
--

‎src/test/regress/sql/subselect.sql

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -890,6 +890,24 @@ fetch backward all in c1;
890890

891891
commit;
892892

893+
--
894+
-- Verify that we correctly flatten cases involving a subquery output
895+
-- expression that doesn't need to be wrapped in a PlaceHolderVar
896+
--
897+
898+
explain (costs off)
899+
select tname, attnamefrom (
900+
select relname::information_schema.sql_identifieras tname,*from
901+
(select*from pg_class c) ss1) ss2
902+
right join pg_attribute aona.attrelid=ss2.oid
903+
where tname='tenk1'and attnum=1;
904+
905+
select tname, attnamefrom (
906+
select relname::information_schema.sql_identifieras tname,*from
907+
(select*from pg_class c) ss1) ss2
908+
right join pg_attribute aona.attrelid=ss2.oid
909+
where tname='tenk1'and attnum=1;
910+
893911
--
894912
-- Tests for CTE inlining behavior
895913
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp