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

Commitcad5692

Browse files
committed
Fix up join removal's interaction with PlaceHolderVars.
The portion of join_is_removable() that checks PlaceHolderVarscan be made a little more accurate and intelligible than it was.The key point is that we can allow join removal even if a PHVmentions the target rel in ph_eval_at, if that mention was onlyadded as a consequence of forcing the PHV up to a join levelthat's at/above the outer join we're trying to get rid of.We can check that by testing for the OJ's relid appearing inph_eval_at, indicating that it's supposed to be evaluated afterthe outer join, plus the existing test that the containedexpression doesn't actually mention the target rel.While here, add an explicit check that there'll be something leftin ph_eval_at after we remove the target rel and OJ relid. Thereis an Assert later on about that, and I'm not too sure that thecase could happen for a PHV satisfying the other constraints,but let's just check. (There was previously a bms_is_subset testthat meant to cover this risk, but it's broken now because itdoesn't account for the fact that we'll also remove the OJ relid.)The real reason for revisiting this code though is that theAssert I left behind in8538519 turns out to be easilyreachable, because if a PHV of this sort appears in an upper-levelqual clause then that clause's clause_relids will include thePHV's ph_eval_at relids. This is a mirage though: we have or soonwill remove these relids from the PHV's ph_eval_at, and thereforethey no longer belong in qual clauses' clause_relids either.Remove that Assert in join_is_removable, and replace the similarone in remove_rel_from_query with code to remove the deleted relidsfrom clause_relids.Per bug #17773 from Robins Tharakan.Discussion:https://postgr.es/m/17773-a592e6cedbc7bac5@postgresql.org
1 parent7ba09ef commitcad5692

File tree

3 files changed

+58
-22
lines changed

3 files changed

+58
-22
lines changed

‎src/backend/optimizer/plan/analyzejoins.c

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -217,28 +217,36 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
217217

218218
/*
219219
* Similarly check that the inner rel isn't needed by any PlaceHolderVars
220-
* that will be used above the join.We only need to fail if such a PHV
221-
*actually references some inner-rel attributes; but the correct check
222-
*forthatis relatively expensive, so we first check against ph_eval_at,
223-
*which must mentiontheinner rel if the PHV uses any inner-rel attrs as
224-
*non-lateral references.Note that if thePHV's syntactic scope is just
225-
*the inner rel, we can'tdrop the rel even if the PHV is variable-free.
220+
* that will be used above the join.The PHV case is a little bit more
221+
*complicated, because PHVs may have been assigned a ph_eval_at location
222+
* thatincludes the innerrel, yet their contained expression might not
223+
*actually referencetheinnerrel (it could be just a constant, for
224+
*instance).If such aPHV is due to be evaluated above the join then it
225+
*needn'tprevent join removal.
226226
*/
227227
foreach(l,root->placeholder_list)
228228
{
229229
PlaceHolderInfo*phinfo= (PlaceHolderInfo*)lfirst(l);
230230

231231
if (bms_overlap(phinfo->ph_lateral,innerrel->relids))
232232
return false;/* it references innerrel laterally */
233-
if (bms_is_subset(phinfo->ph_needed,inputrelids))
234-
continue;/* PHV is not used above the join */
235233
if (!bms_overlap(phinfo->ph_eval_at,innerrel->relids))
236234
continue;/* it definitely doesn't reference innerrel */
237-
if (bms_is_subset(phinfo->ph_eval_at,innerrel->relids))
235+
if (bms_is_subset(phinfo->ph_needed,inputrelids))
236+
continue;/* PHV is not used above the join */
237+
if (!bms_is_member(sjinfo->ojrelid,phinfo->ph_eval_at))
238+
return false;/* it has to be evaluated below the join */
239+
240+
/*
241+
* We need to be sure there will still be a place to evaluate the PHV
242+
* if we remove the join, ie that ph_eval_at wouldn't become empty.
243+
*/
244+
if (!bms_overlap(sjinfo->min_lefthand,phinfo->ph_eval_at))
238245
return false;/* there isn't any other place to eval PHV */
246+
/* Check contained expression last, since this is a bit expensive */
239247
if (bms_overlap(pull_varnos(root, (Node*)phinfo->ph_var->phexpr),
240248
innerrel->relids))
241-
return false;/*it does reference innerrel */
249+
return false;/*contained expression references innerrel */
242250
}
243251

244252
/*
@@ -270,15 +278,7 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
270278
* be from WHERE, for example).
271279
*/
272280
if (RINFO_IS_PUSHED_DOWN(restrictinfo,joinrelids))
273-
{
274-
/*
275-
* If such a clause actually references the inner rel then join
276-
* removal has to be disallowed. The previous attr_needed checks
277-
* should have caught this, though.
278-
*/
279-
Assert(!bms_is_member(innerrelid,restrictinfo->clause_relids));
280281
continue;/* ignore; not useful here */
281-
}
282282

283283
/* Ignore if it's not a mergejoinable clause */
284284
if (!restrictinfo->can_join||
@@ -465,13 +465,21 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
465465
*/
466466
if (bms_is_member(ojrelid,rinfo->required_relids))
467467
{
468-
/* Recheck that qual doesn't actually reference the target rel */
469-
Assert(!bms_is_member(relid,rinfo->clause_relids));
470-
471468
/*
472-
* The required_relids probably aren't shared with anything else,
469+
* There might be references to relid or ojrelid in the
470+
* clause_relids as a consequence of PHVs having ph_eval_at sets
471+
* that include those. We already checked above that any such PHV
472+
* is safe, so we can just drop those references.
473+
*
474+
* The clause_relids probably aren't shared with anything else,
473475
* but let's copy them just to be sure.
474476
*/
477+
rinfo->clause_relids=bms_copy(rinfo->clause_relids);
478+
rinfo->clause_relids=bms_del_member(rinfo->clause_relids,
479+
relid);
480+
rinfo->clause_relids=bms_del_member(rinfo->clause_relids,
481+
ojrelid);
482+
/* Likewise for required_relids */
475483
rinfo->required_relids=bms_copy(rinfo->required_relids);
476484
rinfo->required_relids=bms_del_member(rinfo->required_relids,
477485
relid);

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5213,6 +5213,26 @@ SELECT q2 FROM
52135213
Index Cond: (id = int8_tbl.q2)
52145214
(5 rows)
52155215

5216+
-- join removal bug #17773: otherwise-removable PHV appears in a qual condition
5217+
EXPLAIN (VERBOSE, COSTS OFF)
5218+
SELECT q2 FROM
5219+
(SELECT q2, 'constant'::text AS x
5220+
FROM int8_tbl LEFT JOIN innertab ON q2 = id) ss
5221+
RIGHT JOIN int4_tbl ON NULL
5222+
WHERE x >= x;
5223+
QUERY PLAN
5224+
------------------------------------------------------
5225+
Nested Loop Left Join
5226+
Output: q2
5227+
Join Filter: NULL::boolean
5228+
Filter: (('constant'::text) >= ('constant'::text))
5229+
-> Seq Scan on public.int4_tbl
5230+
Output: int4_tbl.f1
5231+
-> Result
5232+
Output: q2, 'constant'::text
5233+
One-Time Filter: false
5234+
(9 rows)
5235+
52165236
rollback;
52175237
-- another join removal bug: we must clean up correctly when removing a PHV
52185238
begin;

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1888,6 +1888,14 @@ SELECT q2 FROM
18881888
FROM int8_tblLEFT JOIN innertabON q2= id) ss
18891889
WHERE COALESCE(dat1,0)= q1;
18901890

1891+
-- join removal bug #17773: otherwise-removable PHV appears in a qual condition
1892+
EXPLAIN (VERBOSE, COSTS OFF)
1893+
SELECT q2FROM
1894+
(SELECT q2,'constant'::textAS x
1895+
FROM int8_tblLEFT JOIN innertabON q2= id) ss
1896+
RIGHT JOIN int4_tblONNULL
1897+
WHERE x>= x;
1898+
18911899
rollback;
18921900

18931901
-- another join removal bug: we must clean up correctly when removing a PHV

0 commit comments

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