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

Commitf50f029

Browse files
committed
Fix thinkos in have_unsafe_outer_join_ref; reduce to Assert check.
Late in the development of commit2489d76, I (tgl) incorrectlyconcluded that the new function have_unsafe_outer_join_ref couldn'tever reach its inner loop. That should be the case if the innerrel's parameterization is based on just one Var, but it could bebased on Vars from several relations, and then not only is theinner loop reachable but it's wrongly coded.Despite those errors, it still appears that the whole thing isredundant given previous join_is_legal checks, so let's arrangeto only run it in assert-enabled builds.Diagnosis and patch by Richard Guo, per fuzz testing by Justin Pryzby.Discussion:https://postgr.es/m/20230212235823.GW1653@telsasoft.com
1 parentb16259b commitf50f029

File tree

3 files changed

+57
-15
lines changed

3 files changed

+57
-15
lines changed

‎src/backend/optimizer/path/joinpath.c

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -378,21 +378,20 @@ allow_star_schema_join(PlannerInfo *root,
378378
* restrictions prevent us from attempting a join that would cause a problem.
379379
* (That's unsurprising, because the code worked before we ever added
380380
* outer-join relids to expression relids.) It still seems worth checking
381-
* as a backstop, but we don't go to a lot of trouble: just reject if the
382-
* unsatisfied part includes any outer-join relids at all.
381+
* as a backstop, but we only do so in assert-enabled builds.
383382
*/
383+
#ifdefUSE_ASSERT_CHECKING
384384
staticinlinebool
385385
have_unsafe_outer_join_ref(PlannerInfo*root,
386386
Relidsouterrelids,
387387
Relidsinner_paramrels)
388388
{
389389
boolresult= false;
390390
Relidsunsatisfied=bms_difference(inner_paramrels,outerrelids);
391+
Relidssatisfied=bms_intersect(inner_paramrels,outerrelids);
391392

392-
if (unlikely(bms_overlap(unsatisfied,root->outer_join_rels)))
393+
if (bms_overlap(unsatisfied,root->outer_join_rels))
393394
{
394-
#ifdefNOT_USED
395-
/* If we ever weaken the join order restrictions, we might need this */
396395
ListCell*lc;
397396

398397
foreach(lc,root->join_info_list)
@@ -401,25 +400,23 @@ have_unsafe_outer_join_ref(PlannerInfo *root,
401400

402401
if (!bms_is_member(sjinfo->ojrelid,unsatisfied))
403402
continue;/* not relevant */
404-
if (bms_overlap(inner_paramrels,sjinfo->min_righthand)||
403+
if (bms_overlap(satisfied,sjinfo->min_righthand)||
405404
(sjinfo->jointype==JOIN_FULL&&
406-
bms_overlap(inner_paramrels,sjinfo->min_lefthand)))
405+
bms_overlap(satisfied,sjinfo->min_lefthand)))
407406
{
408407
result= true;/* doesn't work */
409408
break;
410409
}
411410
}
412-
#else
413-
/* For now, if we do see an overlap, just assume it's trouble */
414-
result= true;
415-
#endif
416411
}
417412

418413
/* Waste no memory when we reject a path here */
419414
bms_free(unsatisfied);
415+
bms_free(satisfied);
420416

421417
returnresult;
422418
}
419+
#endif/* USE_ASSERT_CHECKING */
423420

424421
/*
425422
* paraminfo_get_equal_hashops
@@ -713,23 +710,25 @@ try_nestloop_path(PlannerInfo *root,
713710
/*
714711
* Check to see if proposed path is still parameterized, and reject if the
715712
* parameterization wouldn't be sensible --- unless allow_star_schema_join
716-
* says to allow it anyway. Also, we must reject ifeither
717-
*have_unsafe_outer_join_ref or have_dangerous_phv don't like the look of
718-
*it, which could only happen if the nestloop isstill parameterized.
713+
* says to allow it anyway. Also, we must reject ifhave_dangerous_phv
714+
*doesn't like the look of it, which could only happen if the nestloop is
715+
* still parameterized.
719716
*/
720717
required_outer=calc_nestloop_required_outer(outerrelids,outer_paramrels,
721718
innerrelids,inner_paramrels);
722719
if (required_outer&&
723720
((!bms_overlap(required_outer,extra->param_source_rels)&&
724721
!allow_star_schema_join(root,outerrelids,inner_paramrels))||
725-
have_unsafe_outer_join_ref(root,outerrelids,inner_paramrels)||
726722
have_dangerous_phv(root,outerrelids,inner_paramrels)))
727723
{
728724
/* Waste no memory when we reject a path here */
729725
bms_free(required_outer);
730726
return;
731727
}
732728

729+
/* If we got past that, we shouldn't have any unsafe outer-join refs */
730+
Assert(!have_unsafe_outer_join_ref(root,outerrelids,inner_paramrels));
731+
733732
/*
734733
* Do a precheck to quickly eliminate obviously-inferior paths. We
735734
* calculate a cheap lower bound on the path's cost and then use

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4653,6 +4653,32 @@ where tt1.f1 = ss1.c0;
46534653
----------
46544654
(0 rows)
46554655

4656+
explain (verbose, costs off)
4657+
select 1 from
4658+
int4_tbl as i4
4659+
inner join
4660+
((select 42 as n from int4_tbl x1 left join int8_tbl x2 on f1 = q1) as ss1
4661+
right join (select 1 as z) as ss2 on true)
4662+
on false,
4663+
lateral (select i4.f1, ss1.n from int8_tbl as i8 limit 1) as ss3;
4664+
QUERY PLAN
4665+
--------------------------
4666+
Result
4667+
Output: 1
4668+
One-Time Filter: false
4669+
(3 rows)
4670+
4671+
select 1 from
4672+
int4_tbl as i4
4673+
inner join
4674+
((select 42 as n from int4_tbl x1 left join int8_tbl x2 on f1 = q1) as ss1
4675+
right join (select 1 as z) as ss2 on true)
4676+
on false,
4677+
lateral (select i4.f1, ss1.n from int8_tbl as i8 limit 1) as ss3;
4678+
?column?
4679+
----------
4680+
(0 rows)
4681+
46564682
--
46574683
-- check a case in which a PlaceHolderVar forces join order
46584684
--

‎src/test/regress/sql/join.sql

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,6 +1609,23 @@ select 1 from
16091609
lateral (selecttt4.f1as c0from text_tblas tt5limit1)as ss1
16101610
wherett1.f1=ss1.c0;
16111611

1612+
explain (verbose, costs off)
1613+
select1from
1614+
int4_tblas i4
1615+
inner join
1616+
((select42as nfrom int4_tbl x1left join int8_tbl x2on f1= q1)as ss1
1617+
right join (select1as z)as ss2on true)
1618+
on false,
1619+
lateral (selecti4.f1,ss1.nfrom int8_tblas i8limit1)as ss3;
1620+
1621+
select1from
1622+
int4_tblas i4
1623+
inner join
1624+
((select42as nfrom int4_tbl x1left join int8_tbl x2on f1= q1)as ss1
1625+
right join (select1as z)as ss2on true)
1626+
on false,
1627+
lateral (selecti4.f1,ss1.nfrom int8_tblas i8limit1)as ss3;
1628+
16121629
--
16131630
-- check a case in which a PlaceHolderVar forces join order
16141631
--

0 commit comments

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