Fix thinko in qual distribution.

tglsfdc · tglsfdc · commit 9f452feeeb83 · 2023-02-04T17:40:35.000-05:00
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
@@ -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
@@ -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
@@ -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
 --
