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

Commit80d9c07

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 parent9fe6319 commit80d9c07

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
@@ -2447,14 +2447,48 @@ pullup_replace_vars_callback(Var *var,
24472447
else
24482448
wrap= false;
24492449
}
2450+
elseif (rcon->wrap_non_vars)
2451+
{
2452+
/* Caller told us to wrap all non-Vars in a PlaceHolderVar */
2453+
wrap= true;
2454+
}
24502455
else
24512456
{
24522457
/*
2453-
* Must wrap, either because we need a place to insert
2454-
* varnullingrels or because caller told us to wrap
2455-
* everything.
2458+
* If the node contains Var(s) or PlaceHolderVar(s) of the
2459+
* subquery being pulled up, and does not contain any
2460+
* non-strict constructs, then instead of adding a PHV on top
2461+
* we can add the required nullingrels to those Vars/PHVs.
2462+
* (This is fundamentally a generalization of the above cases
2463+
* for bare Vars and PHVs.)
2464+
*
2465+
* This test is somewhat expensive, but it avoids pessimizing
2466+
* the plan in cases where the nullingrels get removed again
2467+
* later by outer join reduction.
2468+
*
2469+
* This analysis could be tighter: in particular, a non-strict
2470+
* construct hidden within a lower-level PlaceHolderVar is not
2471+
* reason to add another PHV. But for now it doesn't seem
2472+
* worth the code to be more exact.
2473+
*
2474+
* For a LATERAL subquery, we have to check the actual var
2475+
* membership of the node, but if it's non-lateral then any
2476+
* level-zero var must belong to the subquery.
24562477
*/
2457-
wrap= true;
2478+
if ((rcon->target_rte->lateral ?
2479+
bms_overlap(pull_varnos(rcon->root,newnode),
2480+
rcon->relids) :
2481+
contain_vars_of_level(newnode,0))&&
2482+
!contain_nonstrict_functions(newnode))
2483+
{
2484+
/* No wrap needed */
2485+
wrap= false;
2486+
}
2487+
else
2488+
{
2489+
/* Else wrap it in a PlaceHolderVar */
2490+
wrap= true;
2491+
}
24582492
}
24592493

24602494
if (wrap)
@@ -2475,33 +2509,41 @@ pullup_replace_vars_callback(Var *var,
24752509
}
24762510
}
24772511

2478-
/* Must adjust varlevelsup if replaced Var is within a subquery */
2479-
if (var->varlevelsup>0)
2480-
IncrementVarSublevelsUp(newnode,var->varlevelsup,0);
2481-
2482-
/* Propagate any varnullingrels into the replacement Var or PHV */
2512+
/* Propagate any varnullingrels into the replacement expression */
24832513
if (var->varnullingrels!=NULL)
24842514
{
24852515
if (IsA(newnode,Var))
24862516
{
24872517
Var*newvar= (Var*)newnode;
24882518

2489-
Assert(newvar->varlevelsup==var->varlevelsup);
2519+
Assert(newvar->varlevelsup==0);
24902520
newvar->varnullingrels=bms_add_members(newvar->varnullingrels,
24912521
var->varnullingrels);
24922522
}
24932523
elseif (IsA(newnode,PlaceHolderVar))
24942524
{
24952525
PlaceHolderVar*newphv= (PlaceHolderVar*)newnode;
24962526

2497-
Assert(newphv->phlevelsup==var->varlevelsup);
2527+
Assert(newphv->phlevelsup==0);
24982528
newphv->phnullingrels=bms_add_members(newphv->phnullingrels,
24992529
var->varnullingrels);
25002530
}
25012531
else
2502-
elog(ERROR,"failed to wrap a non-Var");
2532+
{
2533+
/* There should be lower-level Vars/PHVs we can modify */
2534+
newnode=add_nulling_relids(newnode,
2535+
NULL,/* modify all Vars/PHVs */
2536+
var->varnullingrels);
2537+
/* Assert we did put the varnullingrels into the expression */
2538+
Assert(bms_is_subset(var->varnullingrels,
2539+
pull_varnos(rcon->root,newnode)));
2540+
}
25032541
}
25042542

2543+
/* Must adjust varlevelsup if replaced Var is within a subquery */
2544+
if (var->varlevelsup>0)
2545+
IncrementVarSublevelsUp(newnode,var->varlevelsup,0);
2546+
25052547
returnnewnode;
25062548
}
25072549

‎src/backend/rewrite/rewriteManip.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,7 +1133,8 @@ AddInvertedQual(Query *parsetree, Node *qual)
11331133
/*
11341134
* add_nulling_relids() finds Vars and PlaceHolderVars that belong to any
11351135
* of the target_relids, and adds added_relids to their varnullingrels
1136-
* and phnullingrels fields.
1136+
* and phnullingrels fields. If target_relids is NULL, all level-zero
1137+
* Vars and PHVs are modified.
11371138
*/
11381139
Node*
11391140
add_nulling_relids(Node*node,
@@ -1162,7 +1163,8 @@ add_nulling_relids_mutator(Node *node,
11621163
Var*var= (Var*)node;
11631164

11641165
if (var->varlevelsup==context->sublevels_up&&
1165-
bms_is_member(var->varno,context->target_relids))
1166+
(context->target_relids==NULL||
1167+
bms_is_member(var->varno,context->target_relids)))
11661168
{
11671169
Relidsnewnullingrels=bms_union(var->varnullingrels,
11681170
context->added_relids);
@@ -1180,7 +1182,8 @@ add_nulling_relids_mutator(Node *node,
11801182
PlaceHolderVar*phv= (PlaceHolderVar*)node;
11811183

11821184
if (phv->phlevelsup==context->sublevels_up&&
1183-
bms_overlap(phv->phrels,context->target_relids))
1185+
(context->target_relids==NULL||
1186+
bms_overlap(phv->phrels,context->target_relids)))
11841187
{
11851188
Relidsnewnullingrels=bms_union(phv->phnullingrels,
11861189
context->added_relids);

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

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

16721672
commit;
1673+
--
1674+
-- Verify that we correctly flatten cases involving a subquery output
1675+
-- expression that doesn't need to be wrapped in a PlaceHolderVar
1676+
--
1677+
explain (costs off)
1678+
select tname, attname from (
1679+
select relname::information_schema.sql_identifier as tname, * from
1680+
(select * from pg_class c) ss1) ss2
1681+
right join pg_attribute a on a.attrelid = ss2.oid
1682+
where tname = 'tenk1' and attnum = 1;
1683+
QUERY PLAN
1684+
--------------------------------------------------------------------------
1685+
Nested Loop
1686+
-> Index Scan using pg_class_relname_nsp_index on pg_class c
1687+
Index Cond: (relname = 'tenk1'::name)
1688+
-> Index Scan using pg_attribute_relid_attnum_index on pg_attribute a
1689+
Index Cond: ((attrelid = c.oid) AND (attnum = 1))
1690+
(5 rows)
1691+
1692+
select tname, attname from (
1693+
select relname::information_schema.sql_identifier as tname, * from
1694+
(select * from pg_class c) ss1) ss2
1695+
right join pg_attribute a on a.attrelid = ss2.oid
1696+
where tname = 'tenk1' and attnum = 1;
1697+
tname | attname
1698+
-------+---------
1699+
tenk1 | unique1
1700+
(1 row)
1701+
16731702
--
16741703
-- Tests for CTE inlining behavior
16751704
--

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

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

877877
commit;
878878

879+
--
880+
-- Verify that we correctly flatten cases involving a subquery output
881+
-- expression that doesn't need to be wrapped in a PlaceHolderVar
882+
--
883+
884+
explain (costs off)
885+
select tname, attnamefrom (
886+
select relname::information_schema.sql_identifieras tname,*from
887+
(select*from pg_class c) ss1) ss2
888+
right join pg_attribute aona.attrelid=ss2.oid
889+
where tname='tenk1'and attnum=1;
890+
891+
select tname, attnamefrom (
892+
select relname::information_schema.sql_identifieras tname,*from
893+
(select*from pg_class c) ss1) ss2
894+
right join pg_attribute aona.attrelid=ss2.oid
895+
where tname='tenk1'and attnum=1;
896+
879897
--
880898
-- Tests for CTE inlining behavior
881899
--

0 commit comments

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