Fix thinko in qual distribution.

deconstruct_distribute tweaks the outer join scope (ojscope)
it passes to distribute_qual_to_rels when considering an outer
join qual that's above potentially-commutable outer joins.
However, if the current join is *not* potentially commutable,
we shouldn't do that.  The argument that distribute_qual_to_rels
will not do something wrong with the bogus ojscope falls flat
if we don't pass it non-null postponed_oj_qual_list.  Moreover,
there's no need to play games in this case since we aren't going
to commute anything.

Per SQLSmith testing by Robins Tharakan.

Discussion: https://postgr.es/m/CAEP4nAw74k4b-=93gmfCNX3MOY3y4uPxqbk_MnCVEpdsqHJVsg@mail.gmail.com

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index a19260f9bc4b009b174783b366ab28d6c4b39407..c301e6dffc41f60c9d7e77ef853c7ed0aa155fa4 100644 (file)
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -1189,23 +1189,8 @@ deconstruct_distribute(PlannerInfo *root, JoinTreeItem *jtitem)
                        if (j->jointype == JOIN_SEMI)
                                ojscope = NULL;
                        else
-                       {
                                ojscope = bms_union(sjinfo->min_lefthand,
                                                                        sjinfo->min_righthand);
-
-                               /*
-                                * Add back any commutable lower OJ relids that were removed
-                                * from min_lefthand or min_righthand, else the ojscope
-                                * cross-check in distribute_qual_to_rels will complain.  If
-                                * any such OJs were removed, we will postpone processing of
-                                * non-degenerate clauses, so this addition doesn't affect
-                                * anything except that cross-check and some Asserts.  Real
-                                * clause positioning decisions will be made later, when we
-                                * revisit the postponed clauses.
-                                */
-                               if (sjinfo->commute_below)
-                                       ojscope = bms_add_members(ojscope, sjinfo->commute_below);
-                       }
                }
                else
                {
@@ -1221,7 +1206,21 @@ deconstruct_distribute(PlannerInfo *root, JoinTreeItem *jtitem)
                 * they will drop down below this join anyway.)
                 */
                if (j->jointype == JOIN_LEFT && sjinfo->lhs_strict)
+               {
                        postponed_oj_qual_list = &jtitem->oj_joinclauses;
+
+                       /*
+                        * Add back any commutable lower OJ relids that were removed from
+                        * min_lefthand or min_righthand, else the ojscope cross-check in
+                        * distribute_qual_to_rels will complain.  Since we are postponing
+                        * processing of non-degenerate clauses, this addition doesn't
+                        * affect anything except that cross-check.  Real clause
+                        * positioning decisions will be made later, when we revisit the
+                        * postponed clauses.
+                        */
+                       if (sjinfo->commute_below)
+                               ojscope = bms_add_members(ojscope, sjinfo->commute_below);
+               }
                else
                        postponed_oj_qual_list = NULL;
 

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index c520839bf7d7b961caa60709ac68fa23faaec0a1..9762952efd21dc76c8ae7700300d74a0a3eb8fd2 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -4451,6 +4451,24 @@ select * from
  doh! | 123 | 456 | hi de ho neighbor |   
 (2 rows)
 
+-- check handling of a variable-free qual for a non-commutable outer join
+explain (costs off)
+select nspname
+from (select 1 as x) ss1
+left join
+( select n.nspname, c.relname
+  from pg_class c left join pg_namespace n on n.oid = c.relnamespace
+  where c.relkind = 'r'
+) ss2 on false;
+           QUERY PLAN           
+--------------------------------
+ Nested Loop Left Join
+   Join Filter: false
+   ->  Result
+   ->  Result
+         One-Time Filter: false
+(5 rows)
+
 --
 -- test for appropriate join order in the presence of lateral references
 --

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index b0e8d559cdb7c895da5fabff87910bd25c231901..3ef29960409541405e7c353ab0d3fe86590ce6b2 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1535,6 +1535,16 @@ select * from
   left join int4_tbl i4
   on i8.q1 = i4.f1;
 
+-- check handling of a variable-free qual for a non-commutable outer join
+explain (costs off)
+select nspname
+from (select 1 as x) ss1
+left join
+( select n.nspname, c.relname
+  from pg_class c left join pg_namespace n on n.oid = c.relnamespace
+  where c.relkind = 'r'
+) ss2 on false;
+
 --
 -- test for appropriate join order in the presence of lateral references
 --