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

Commit78883cd

Browse files
committed
Avoid mislabeling of lateral references, redux.
As I'd feared, commit5c9d863 was still a few bricks shy of a load.We can't just leave pulled-up lateral-reference Vars with no newnullingrels: we have to carefully compute what subset of theto-be-replaced Var's nullingrels apply to them, else we still get"wrong varnullingrels" errors. This is a bit tedious, but it lookslike we can use the nullingrel data this patch computes for otherpurposes, enabling better optimization. We don't want to injectunnecessary plan changes into stable branches though, so leave thatidea for a later HEAD-only patch.Patch by me, but thanks to Richard Guo for devising a test case thatbroke5c9d863, and for preliminary investigation about how to fixit. As before, back-patch to v16.Discussion:https://postgr.es/m/E1tGn4j-0003zi-MP@gemulon.postgresql.org
1 parent72822a9 commit78883cd

File tree

4 files changed

+233
-5
lines changed

4 files changed

+233
-5
lines changed

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

Lines changed: 166 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,26 @@
4242
#include"rewrite/rewriteManip.h"
4343

4444

45+
typedefstructnullingrel_info
46+
{
47+
/*
48+
* For each leaf RTE, nullingrels[rti] is the set of relids of outer joins
49+
* that potentially null that RTE.
50+
*/
51+
Relids*nullingrels;
52+
/* Length of range table (maximum index in nullingrels[]) */
53+
intrtlength;/* used only for assertion checks */
54+
}nullingrel_info;
55+
4556
typedefstructpullup_replace_vars_context
4657
{
4758
PlannerInfo*root;
4859
List*targetlist;/* tlist of subquery being pulled up */
4960
RangeTblEntry*target_rte;/* RTE of subquery */
5061
Relidsrelids;/* relids within subquery, as numbered after
5162
* pullup (set only if target_rte->lateral) */
63+
nullingrel_info*nullinfo;/* per-RTE nullingrel info (set only if
64+
* target_rte->lateral) */
5265
bool*outer_hasSubLinks;/* -> outer query's hasSubLinks */
5366
intvarno;/* varno of subquery */
5467
boolwrap_non_vars;/* do we need all non-Var outputs to be PHVs? */
@@ -142,6 +155,9 @@ static void substitute_phv_relids(Node *node,
142155
staticvoidfix_append_rel_relids(PlannerInfo*root,intvarno,
143156
Relidssubrelids);
144157
staticNode*find_jointree_node_for_rel(Node*jtnode,intrelid);
158+
staticnullingrel_info*get_nullingrels(Query*parse);
159+
staticvoidget_nullingrels_recurse(Node*jtnode,Relidsupper_nullingrels,
160+
nullingrel_info*info);
145161

146162

147163
/*
@@ -1259,10 +1275,16 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
12591275
rvcontext.targetlist=subquery->targetList;
12601276
rvcontext.target_rte=rte;
12611277
if (rte->lateral)
1278+
{
12621279
rvcontext.relids=get_relids_in_jointree((Node*)subquery->jointree,
12631280
true, true);
1264-
else/* won't need relids */
1281+
rvcontext.nullinfo=get_nullingrels(parse);
1282+
}
1283+
else/* won't need these values */
1284+
{
12651285
rvcontext.relids=NULL;
1286+
rvcontext.nullinfo=NULL;
1287+
}
12661288
rvcontext.outer_hasSubLinks=&parse->hasSubLinks;
12671289
rvcontext.varno=varno;
12681290
/* this flag will be set below, if needed */
@@ -1724,6 +1746,9 @@ is_simple_subquery(PlannerInfo *root, Query *subquery, RangeTblEntry *rte,
17241746
* such refs to be wrapped in PlaceHolderVars, even when they're below
17251747
* the nearest outer join?But it's a pretty hokey usage, so not
17261748
* clear this is worth sweating over.)
1749+
*
1750+
* If you change this, see also the comments about lateral references
1751+
* in pullup_replace_vars_callback().
17271752
*/
17281753
if (lowest_outer_join!=NULL)
17291754
{
@@ -1808,7 +1833,8 @@ pull_up_simple_values(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte)
18081833
rvcontext.root=root;
18091834
rvcontext.targetlist=tlist;
18101835
rvcontext.target_rte=rte;
1811-
rvcontext.relids=NULL;
1836+
rvcontext.relids=NULL;/* can't be any lateral references here */
1837+
rvcontext.nullinfo=NULL;
18121838
rvcontext.outer_hasSubLinks=&parse->hasSubLinks;
18131839
rvcontext.varno=varno;
18141840
rvcontext.wrap_non_vars= false;
@@ -1970,9 +1996,10 @@ pull_up_constant_function(PlannerInfo *root, Node *jtnode,
19701996
/*
19711997
* Since this function was reduced to a Const, it doesn't contain any
19721998
* lateral references, even if it's marked as LATERAL. This means we
1973-
* don't need to fill relids.
1999+
* don't need to fill relids or nullinfo.
19742000
*/
19752001
rvcontext.relids=NULL;
2002+
rvcontext.nullinfo=NULL;
19762003

19772004
rvcontext.outer_hasSubLinks=&parse->hasSubLinks;
19782005
rvcontext.varno= ((RangeTblRef*)jtnode)->rtindex;
@@ -2681,9 +2708,52 @@ pullup_replace_vars_callback(Var *var,
26812708
{
26822709
/*
26832710
* There should be Vars/PHVs within the expression that we can
2684-
* modify. Per above discussion, modify only Vars/PHVs of the
2685-
* subquery, not lateral references.
2711+
* modify. Vars/PHVs of the subquery should have the full
2712+
* var->varnullingrels added to them, but if there are lateral
2713+
* references within the expression, those must be marked with
2714+
* only the nullingrels that potentially apply to them. (This
2715+
* corresponds to the fact that the expression will now be
2716+
* evaluated at the join level of the Var that we are replacing:
2717+
* the lateral references may have bubbled up through fewer outer
2718+
* joins than the subquery's Vars have. Per the discussion above,
2719+
* we'll still get the right answers.) That relid set could be
2720+
* different for different lateral relations, so we have to do
2721+
* this work for each one.
2722+
*
2723+
* (Currently, the restrictions in is_simple_subquery() mean that
2724+
* at most we have to remove the lowest outer join's relid from
2725+
* the nullingrels of a lateral reference. However, we might
2726+
* relax those restrictions someday, so let's do this right.)
26862727
*/
2728+
if (rcon->target_rte->lateral)
2729+
{
2730+
nullingrel_info*nullinfo=rcon->nullinfo;
2731+
Relidslvarnos;
2732+
intlvarno;
2733+
2734+
/*
2735+
* Identify lateral varnos used within newnode. We must do
2736+
* this before injecting var->varnullingrels into the tree.
2737+
*/
2738+
lvarnos=pull_varnos(rcon->root,newnode);
2739+
lvarnos=bms_del_members(lvarnos,rcon->relids);
2740+
/* For each one, add relevant nullingrels if any */
2741+
lvarno=-1;
2742+
while ((lvarno=bms_next_member(lvarnos,lvarno)) >=0)
2743+
{
2744+
Relidslnullingrels;
2745+
2746+
Assert(lvarno>0&&lvarno <=nullinfo->rtlength);
2747+
lnullingrels=bms_intersect(var->varnullingrels,
2748+
nullinfo->nullingrels[lvarno]);
2749+
if (!bms_is_empty(lnullingrels))
2750+
newnode=add_nulling_relids(newnode,
2751+
bms_make_singleton(lvarno),
2752+
lnullingrels);
2753+
}
2754+
}
2755+
2756+
/* Finally, deal with Vars/PHVs of the subquery itself */
26872757
newnode=add_nulling_relids(newnode,
26882758
rcon->relids,
26892759
var->varnullingrels);
@@ -4121,3 +4191,94 @@ find_jointree_node_for_rel(Node *jtnode, int relid)
41214191
(int)nodeTag(jtnode));
41224192
returnNULL;
41234193
}
4194+
4195+
/*
4196+
* get_nullingrels: collect info about which outer joins null which relations
4197+
*
4198+
* The result struct contains, for each leaf relation used in the query,
4199+
* the set of relids of outer joins that potentially null that rel.
4200+
*/
4201+
staticnullingrel_info*
4202+
get_nullingrels(Query*parse)
4203+
{
4204+
nullingrel_info*result=palloc_object(nullingrel_info);
4205+
4206+
result->rtlength=list_length(parse->rtable);
4207+
result->nullingrels=palloc0_array(Relids,result->rtlength+1);
4208+
get_nullingrels_recurse((Node*)parse->jointree,NULL,result);
4209+
returnresult;
4210+
}
4211+
4212+
/*
4213+
* Recursive guts of get_nullingrels().
4214+
*
4215+
* Note: at any recursion level, the passed-down upper_nullingrels must be
4216+
* treated as a constant, but it can be stored directly into *info
4217+
* if we're at leaf level. Upper recursion levels do not free their mutated
4218+
* copies of the nullingrels, because those are probably referenced by
4219+
* at least one leaf rel.
4220+
*/
4221+
staticvoid
4222+
get_nullingrels_recurse(Node*jtnode,Relidsupper_nullingrels,
4223+
nullingrel_info*info)
4224+
{
4225+
if (jtnode==NULL)
4226+
return;
4227+
if (IsA(jtnode,RangeTblRef))
4228+
{
4229+
intvarno= ((RangeTblRef*)jtnode)->rtindex;
4230+
4231+
Assert(varno>0&&varno <=info->rtlength);
4232+
info->nullingrels[varno]=upper_nullingrels;
4233+
}
4234+
elseif (IsA(jtnode,FromExpr))
4235+
{
4236+
FromExpr*f= (FromExpr*)jtnode;
4237+
ListCell*l;
4238+
4239+
foreach(l,f->fromlist)
4240+
{
4241+
get_nullingrels_recurse(lfirst(l),upper_nullingrels,info);
4242+
}
4243+
}
4244+
elseif (IsA(jtnode,JoinExpr))
4245+
{
4246+
JoinExpr*j= (JoinExpr*)jtnode;
4247+
Relidslocal_nullingrels;
4248+
4249+
switch (j->jointype)
4250+
{
4251+
caseJOIN_INNER:
4252+
get_nullingrels_recurse(j->larg,upper_nullingrels,info);
4253+
get_nullingrels_recurse(j->rarg,upper_nullingrels,info);
4254+
break;
4255+
caseJOIN_LEFT:
4256+
caseJOIN_SEMI:
4257+
caseJOIN_ANTI:
4258+
local_nullingrels=bms_add_member(bms_copy(upper_nullingrels),
4259+
j->rtindex);
4260+
get_nullingrels_recurse(j->larg,upper_nullingrels,info);
4261+
get_nullingrels_recurse(j->rarg,local_nullingrels,info);
4262+
break;
4263+
caseJOIN_FULL:
4264+
local_nullingrels=bms_add_member(bms_copy(upper_nullingrels),
4265+
j->rtindex);
4266+
get_nullingrels_recurse(j->larg,local_nullingrels,info);
4267+
get_nullingrels_recurse(j->rarg,local_nullingrels,info);
4268+
break;
4269+
caseJOIN_RIGHT:
4270+
local_nullingrels=bms_add_member(bms_copy(upper_nullingrels),
4271+
j->rtindex);
4272+
get_nullingrels_recurse(j->larg,local_nullingrels,info);
4273+
get_nullingrels_recurse(j->rarg,upper_nullingrels,info);
4274+
break;
4275+
default:
4276+
elog(ERROR,"unrecognized join type: %d",
4277+
(int)j->jointype);
4278+
break;
4279+
}
4280+
}
4281+
else
4282+
elog(ERROR,"unrecognized node type: %d",
4283+
(int)nodeTag(jtnode));
4284+
}

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1797,6 +1797,57 @@ order by t1.ten;
17971797
9 | 18000
17981798
(10 rows)
17991799

1800+
explain (verbose, costs off)
1801+
select t1.q1, x from
1802+
int8_tbl t1 left join
1803+
(int8_tbl t2 left join
1804+
lateral (select t2.q1+t3.q1 as x, * from int8_tbl t3) t3 on t2.q2 = t3.q2)
1805+
on t1.q2 = t2.q2
1806+
order by 1, 2;
1807+
QUERY PLAN
1808+
--------------------------------------------------------
1809+
Sort
1810+
Output: t1.q1, ((t2.q1 + t3.q1))
1811+
Sort Key: t1.q1, ((t2.q1 + t3.q1))
1812+
-> Hash Left Join
1813+
Output: t1.q1, (t2.q1 + t3.q1)
1814+
Hash Cond: (t2.q2 = t3.q2)
1815+
-> Hash Left Join
1816+
Output: t1.q1, t2.q1, t2.q2
1817+
Hash Cond: (t1.q2 = t2.q2)
1818+
-> Seq Scan on public.int8_tbl t1
1819+
Output: t1.q1, t1.q2
1820+
-> Hash
1821+
Output: t2.q1, t2.q2
1822+
-> Seq Scan on public.int8_tbl t2
1823+
Output: t2.q1, t2.q2
1824+
-> Hash
1825+
Output: t3.q1, t3.q2
1826+
-> Seq Scan on public.int8_tbl t3
1827+
Output: t3.q1, t3.q2
1828+
(19 rows)
1829+
1830+
select t1.q1, x from
1831+
int8_tbl t1 left join
1832+
(int8_tbl t2 left join
1833+
lateral (select t2.q1+t3.q1 as x, * from int8_tbl t3) t3 on t2.q2 = t3.q2)
1834+
on t1.q2 = t2.q2
1835+
order by 1, 2;
1836+
q1 | x
1837+
------------------+------------------
1838+
123 | 246
1839+
123 | 246
1840+
123 | 4567890123456912
1841+
123 | 4567890123456912
1842+
123 | 9135780246913578
1843+
4567890123456789 | 246
1844+
4567890123456789 | 4567890123456912
1845+
4567890123456789 | 4567890123456912
1846+
4567890123456789 | 9135780246913578
1847+
4567890123456789 | 9135780246913578
1848+
4567890123456789 | 9135780246913578
1849+
(11 rows)
1850+
18001851
--
18011852
-- Tests for CTE inlining behavior
18021853
--

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,21 @@ select t1.ten, sum(x) from
924924
group byt1.ten
925925
order byt1.ten;
926926

927+
explain (verbose, costs off)
928+
selectt1.q1, xfrom
929+
int8_tbl t1left join
930+
(int8_tbl t2left join
931+
lateral (selectt2.q1+t3.q1as x,*from int8_tbl t3) t3ont2.q2=t3.q2)
932+
ont1.q2=t2.q2
933+
order by1,2;
934+
935+
selectt1.q1, xfrom
936+
int8_tbl t1left join
937+
(int8_tbl t2left join
938+
lateral (selectt2.q1+t3.q1as x,*from int8_tbl t3) t3ont2.q2=t3.q2)
939+
ont1.q2=t2.q2
940+
order by1,2;
941+
927942
--
928943
-- Tests for CTE inlining behavior
929944
--

‎src/tools/pgindent/typedefs.list

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3639,6 +3639,7 @@ nodeitem
36393639
normal_rand_fctx
36403640
nsphash_hash
36413641
ntile_context
3642+
nullingrel_info
36423643
numeric
36433644
object_access_hook_type
36443645
object_access_hook_type_str

0 commit comments

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