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

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index e0754165340aa315ef8e062bad8056a41bcc9afd..62c1d7f7fc108e84951491b0b21a8e5a6912abf5 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -2274,6 +2274,7 @@ create_nestloop_plan(PlannerInfo *root,
    if (IS_OUTER_JOIN(best_path->jointype))
    {
        extract_actual_join_clauses(joinrestrictclauses,
+                                   best_path->path.parent->relids,
                                    &joinclauses, &otherclauses);
    }
    else
@@ -2374,6 +2375,7 @@ create_mergejoin_plan(PlannerInfo *root,
    if (IS_OUTER_JOIN(best_path->jpath.jointype))
    {
        extract_actual_join_clauses(joinclauses,
+                                   best_path->jpath.path.parent->relids,
                                    &joinclauses, &otherclauses);
    }
    else
@@ -2657,6 +2659,7 @@ create_hashjoin_plan(PlannerInfo *root,
    if (IS_OUTER_JOIN(best_path->jpath.jointype))
    {
        extract_actual_join_clauses(joinclauses,
+                                   best_path->jpath.path.parent->relids,
                                    &joinclauses, &otherclauses);
    }
    else

diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index 95037a7c617287eaff781f0291d4b7e40e5995ad..1567ea79ce97e353b604080ddb86bed5678e1583 100644 (file)
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -418,6 +418,7 @@ extract_actual_clauses(List *restrictinfo_list,
  */
 void
 extract_actual_join_clauses(List *restrictinfo_list,
+                           Relids joinrelids,
                            List **joinquals,
                            List **otherquals)
 {
@@ -432,7 +433,15 @@ extract_actual_join_clauses(List *restrictinfo_list,
 
        Assert(IsA(rinfo, RestrictInfo));
 
-       if (rinfo->is_pushed_down)
+       /*
+        * We must check both is_pushed_down and required_relids, since an
+        * outer-join clause that's been pushed down to some lower join level
+        * via path parameterization will not be marked is_pushed_down;
+        * nonetheless, it must be treated as a filter clause not a join
+        * clause so far as the lower join level is concerned.
+        */
+       if (rinfo->is_pushed_down ||
+           !bms_is_subset(rinfo->required_relids, joinrelids))
        {
            if (!rinfo->pseudoconstant)
                *otherquals = lappend(*otherquals, rinfo->clause);

diff --git a/src/include/optimizer/restrictinfo.h b/src/include/optimizer/restrictinfo.h
index 2c08339a622c713a4afa184239f51c403d1b5319..954459e968982e8c9aff461faad99ce08aabc57a 100644 (file)
--- a/src/include/optimizer/restrictinfo.h
+++ b/src/include/optimizer/restrictinfo.h
@@ -36,6 +36,7 @@ extern List *get_all_actual_clauses(List *restrictinfo_list);
 extern List *extract_actual_clauses(List *restrictinfo_list,
                       bool pseudoconstant);
 extern void extract_actual_join_clauses(List *restrictinfo_list,
+                           Relids joinrelids,
                            List **joinquals,
                            List **otherquals);
 extern bool join_clause_is_movable_to(RestrictInfo *rinfo, RelOptInfo *baserel);

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 38c77690db281e6f2d96f1f99226277c46681285..d3647c208cc7b0955cb7217209a9deeed7974b63 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -3349,6 +3349,33 @@ order by fault;
          | 123 |   122
 (1 row)
 
+explain (costs off)
+select * from
+(values (1, array[10,20]), (2, array[20,30])) as v1(v1x,v1ys)
+left join (values (1, 10), (2, 20)) as v2(v2x,v2y) on v2x = v1x
+left join unnest(v1ys) as u1(u1y) on u1y = v2y;
+                         QUERY PLAN                          
+-------------------------------------------------------------
+ Nested Loop Left Join
+   ->  Values Scan on "*VALUES*"
+   ->  Hash Right Join
+         Hash Cond: (u1.u1y = "*VALUES*_1".column2)
+         Filter: ("*VALUES*_1".column1 = "*VALUES*".column1)
+         ->  Function Scan on unnest u1
+         ->  Hash
+               ->  Values Scan on "*VALUES*_1"
+(8 rows)
+
+select * from
+(values (1, array[10,20]), (2, array[20,30])) as v1(v1x,v1ys)
+left join (values (1, 10), (2, 20)) as v2(v2x,v2y) on v2x = v1x
+left join unnest(v1ys) as u1(u1y) on u1y = v2y;
+ v1x |  v1ys   | v2x | v2y | u1y 
+-----+---------+-----+-----+-----
+   1 | {10,20} |   1 |  10 |  10
+   2 | {20,30} |   2 |  20 |  20
+(2 rows)
+
 --
 -- test handling of potential equivalence clauses above outer joins
 --

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index b5968c4adefed6c78b33bb72b5cbae6b8d057e69..e24459b15aea1c521e8b42a81cf15b08f8b04cf0 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1019,6 +1019,17 @@ select * from
 where fault = 122
 order by fault;
 
+explain (costs off)
+select * from
+(values (1, array[10,20]), (2, array[20,30])) as v1(v1x,v1ys)
+left join (values (1, 10), (2, 20)) as v2(v2x,v2y) on v2x = v1x
+left join unnest(v1ys) as u1(u1y) on u1y = v2y;
+
+select * from
+(values (1, array[10,20]), (2, array[20,30])) as v1(v1x,v1ys)
+left join (values (1, 10), (2, 20)) as v2(v2x,v2y) on v2x = v1x
+left join unnest(v1ys) as u1(u1y) on u1y = v2y;
+
 --
 -- test handling of potential equivalence clauses above outer joins
 --