Skip to content

Commit e5d8399

Browse files
tglsfdctglsfdc
committed
Fix incorrect handling of join clauses pushed into parameterized paths.
In some cases a clause attached to an outer join can be pushed down into the outer join's RHS even though the clause is not degenerate --- this can happen if we choose to make a parameterized path for the RHS. If the clause ends up attached to a lower outer join, we'd misclassify it as being a "join filter" not a plain "filter" condition at that node, leading to wrong query results. To fix, teach extract_actual_join_clauses to examine each join clause's required_relids, not just its is_pushed_down flag. (The latter now seems vestigial, or at least in need of rethinking, but we won't do anything so invasive as redefining it in a bug-fix patch.) This has been wrong since we introduced parameterized paths in 9.2, though it's evidently hard to hit given the lack of previous reports. The test case used here involves a lateral function call, and I think that a lateral reference may be required to get the planner to select a broken plan; though I wouldn't swear to that. In any case, even if LATERAL is needed to trigger the bug, it still affects all supported branches, so back-patch to all. Per report from Andreas Karlsson. Thanks to Andrew Gierth for preliminary investigation. Discussion: https://postgr.es/m/f8128b11-c5bf-3539-48cd-234178b2314d@proxel.se
1 parent 79b2e52 commit e5d8399

File tree

5 files changed

+52
-1
lines changed

5 files changed

+52
-1
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3802,6 +3802,7 @@ create_nestloop_plan(PlannerInfo *root,
38023802
if (IS_OUTER_JOIN(best_path->jointype))
38033803
{
38043804
extract_actual_join_clauses(joinrestrictclauses,
3805+
best_path->path.parent->relids,
38053806
&joinclauses, &otherclauses);
38063807
}
38073808
else
@@ -3917,6 +3918,7 @@ create_mergejoin_plan(PlannerInfo *root,
39173918
if (IS_OUTER_JOIN(best_path->jpath.jointype))
39183919
{
39193920
extract_actual_join_clauses(joinclauses,
3921+
best_path->jpath.path.parent->relids,
39203922
&joinclauses, &otherclauses);
39213923
}
39223924
else
@@ -4213,6 +4215,7 @@ create_hashjoin_plan(PlannerInfo *root,
42134215
if (IS_OUTER_JOIN(best_path->jpath.jointype))
42144216
{
42154217
extract_actual_join_clauses(joinclauses,
4218+
best_path->jpath.path.parent->relids,
42164219
&joinclauses, &otherclauses);
42174220
}
42184221
else

‎src/backend/optimizer/util/restrictinfo.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@ extract_actual_clauses(List *restrictinfo_list,
381381
*/
382382
void
383383
extract_actual_join_clauses(List *restrictinfo_list,
384+
Relids joinrelids,
384385
List **joinquals,
385386
List **otherquals)
386387
{
@@ -393,7 +394,15 @@ extract_actual_join_clauses(List *restrictinfo_list,
393394
{
394395
RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);
395396

396-
if (rinfo->is_pushed_down)
397+
/*
398+
* We must check both is_pushed_down and required_relids, since an
399+
* outer-join clause that's been pushed down to some lower join level
400+
* via path parameterization will not be marked is_pushed_down;
401+
* nonetheless, it must be treated as a filter clause not a join
402+
* clause so far as the lower join level is concerned.
403+
*/
404+
if (rinfo->is_pushed_down ||
405+
!bms_is_subset(rinfo->required_relids, joinrelids))
397406
{
398407
if (!rinfo->pseudoconstant)
399408
*otherquals = lappend(*otherquals, rinfo->clause);

‎src/include/optimizer/restrictinfo.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ extern List *get_actual_clauses(List *restrictinfo_list);
3636
extern List *extract_actual_clauses(List *restrictinfo_list,
3737
bool pseudoconstant);
3838
extern void extract_actual_join_clauses(List *restrictinfo_list,
39+
Relids joinrelids,
3940
List **joinquals,
4041
List **otherquals);
4142
extern bool join_clause_is_movable_to(RestrictInfo *rinfo, RelOptInfo *baserel);

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3371,6 +3371,33 @@ order by fault;
33713371
| 123 | 122
33723372
(1 row)
33733373

3374+
explain (costs off)
3375+
select * from
3376+
(values (1, array[10,20]), (2, array[20,30])) as v1(v1x,v1ys)
3377+
left join (values (1, 10), (2, 20)) as v2(v2x,v2y) on v2x = v1x
3378+
left join unnest(v1ys) as u1(u1y) on u1y = v2y;
3379+
QUERY PLAN
3380+
-------------------------------------------------------------
3381+
Nested Loop Left Join
3382+
-> Values Scan on "*VALUES*"
3383+
-> Hash Right Join
3384+
Hash Cond: (u1.u1y = "*VALUES*_1".column2)
3385+
Filter: ("*VALUES*_1".column1 = "*VALUES*".column1)
3386+
-> Function Scan on unnest u1
3387+
-> Hash
3388+
-> Values Scan on "*VALUES*_1"
3389+
(8 rows)
3390+
3391+
select * from
3392+
(values (1, array[10,20]), (2, array[20,30])) as v1(v1x,v1ys)
3393+
left join (values (1, 10), (2, 20)) as v2(v2x,v2y) on v2x = v1x
3394+
left join unnest(v1ys) as u1(u1y) on u1y = v2y;
3395+
v1x | v1ys | v2x | v2y | u1y
3396+
-----+---------+-----+-----+-----
3397+
1 | {10,20} | 1 | 10 | 10
3398+
2 | {20,30} | 2 | 20 | 20
3399+
(2 rows)
3400+
33743401
--
33753402
-- test handling of potential equivalence clauses above outer joins
33763403
--

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,6 +1028,17 @@ select * from
10281028
where fault = 122
10291029
order by fault;
10301030

1031+
explain (costs off)
1032+
select * from
1033+
(values (1, array[10,20]), (2, array[20,30])) as v1(v1x,v1ys)
1034+
left join (values (1, 10), (2, 20)) as v2(v2x,v2y) on v2x = v1x
1035+
left join unnest(v1ys) as u1(u1y) on u1y = v2y;
1036+
1037+
select * from
1038+
(values (1, array[10,20]), (2, array[20,30])) as v1(v1x,v1ys)
1039+
left join (values (1, 10), (2, 20)) as v2(v2x,v2y) on v2x = v1x
1040+
left join unnest(v1ys) as u1(u1y) on u1y = v2y;
1041+
10311042
--
10321043
-- test handling of potential equivalence clauses above outer joins
10331044
--

0 commit comments

Comments
 (0)