Do assorted mop-up in the planner.

Remove RestrictInfo.nullable_relids, along with a good deal of
infrastructure that calculated it.  One use-case for it was in
join_clause_is_movable_to, but we can now replace that usage with
a check to see if the clause's relids include any outer join
that can null the target relation.  The other use-case was in
join_clause_is_movable_into, but that test can just be dropped
entirely now that the clause's relids include outer joins.
Furthermore, join_clause_is_movable_into should now be
accurate enough that it will accept anything returned by
generate_join_implied_equalities, so we can restore the Assert
that was diked out in commit 95f4e59c3.

Remove the outerjoin_delayed mechanism.  We needed this before to
prevent quals from getting evaluated below outer joins that should
null some of their vars.  Now that we consider varnullingrels while
placing quals, that's taken care of automatically, so throw the
whole thing away.

Teach remove_useless_result_rtes to also remove useless FromExprs.
Having done that, the delay_upper_joins flag serves no purpose any
more and we can remove it, largely reverting 11086f2f2.

Use constant TRUE for "dummy" clauses when throwing back outer joins.
This improves on a hack I introduced in commit 6a6522529.  If we
have a left-join clause l.x = r.y, and a WHERE clause l.x = constant,
we generate r.y = constant and then don't really have a need for the
join clause.  But we must throw the join clause back anyway after
marking it redundant, so that the join search heuristics won't think
this is a clauseless join and avoid it.  That was a kluge introduced
under time pressure, and after looking at it I thought of a better
way: let's just introduce constant-TRUE "join clauses" instead,
and get rid of them at the end.  This improves the generated plans for
such cases by not having to test a redundant join clause.  We can also
get rid of the ugly hack used to mark such clauses as redundant for
selectivity estimation.

Patch by me; thanks to Richard Guo for review.

Discussion: https://postgr.es/m/830269.1656693747@sss.pgh.pa.us

6 files changed, 72 insertions, 191 deletions

diff --git a/src/backend/optimizer/util/appendinfo.c b/src/backend/optimizer/util/appendinfo.c
index d449b5c2746..9d377385f1f 100644
--- a/src/backend/optimizer/util/appendinfo.c
+++ b/src/backend/optimizer/util/appendinfo.c
@@ -467,9 +467,6 @@ adjust_appendrel_attrs_mutator(Node *node,
 		newinfo->outer_relids = adjust_child_relids(oldinfo->outer_relids,
 													context->nappinfos,
 													context->appinfos);
-		newinfo->nullable_relids = adjust_child_relids(oldinfo->nullable_relids,
-													   context->nappinfos,
-													   context->appinfos);
 		newinfo->left_relids = adjust_child_relids(oldinfo->left_relids,
 												   context->nappinfos,
 												   context->appinfos);
diff --git a/src/backend/optimizer/util/inherit.c b/src/backend/optimizer/util/inherit.c
index bf3ea26cf4b..bae9688e461 100644
--- a/src/backend/optimizer/util/inherit.c
+++ b/src/backend/optimizer/util/inherit.c
@@ -894,10 +894,9 @@ apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel,
 								 make_restrictinfo(root,
 												   (Expr *) onecq,
 												   rinfo->is_pushed_down,
-												   rinfo->outerjoin_delayed,
 												   pseudoconstant,
 												   rinfo->security_level,
-												   NULL, NULL, NULL));
+												   NULL, NULL));
 			/* track minimum security level among child quals */
 			cq_min_security = Min(cq_min_security, rinfo->security_level);
 		}
@@ -930,9 +929,9 @@ apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel,
 				/* not likely that we'd see constants here, so no check */
 				childquals = lappend(childquals,
 									 make_restrictinfo(root, qual,
-													   true, false, false,
+													   true, false,
 													   security_level,
-													   NULL, NULL, NULL));
+													   NULL, NULL));
 				cq_min_security = Min(cq_min_security, security_level);
 			}
 			security_level++;
diff --git a/src/backend/optimizer/util/orclauses.c b/src/backend/optimizer/util/orclauses.c
index abc994dbf2e..85ecdfc14f2 100644
--- a/src/backend/optimizer/util/orclauses.c
+++ b/src/backend/optimizer/util/orclauses.c
@@ -98,18 +98,13 @@ extract_restriction_or_clauses(PlannerInfo *root)
 		 * joinclause that is considered safe to move to this rel by the
 		 * parameterized-path machinery, even though what we are going to do
 		 * with it is not exactly a parameterized path.
-		 *
-		 * However, it seems best to ignore clauses that have been marked
-		 * redundant (by setting norm_selec > 1).  That likely can't happen
-		 * for OR clauses, but let's be safe.
 		 */
 		foreach(lc, rel->joininfo)
 		{
 			RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
 
 			if (restriction_is_or_clause(rinfo) &&
-				join_clause_is_movable_to(rinfo, rel) &&
-				rinfo->norm_selec <= 1)
+				join_clause_is_movable_to(rinfo, rel))
 			{
 				/* Try to extract a qual for this rel only */
 				Expr	   *orclause = extract_or_clause(rinfo, rel);
@@ -272,10 +267,8 @@ consider_new_or_clause(PlannerInfo *root, RelOptInfo *rel,
 								 orclause,
 								 true,
 								 false,
-								 false,
 								 join_or_rinfo->security_level,
 								 NULL,
-								 NULL,
 								 NULL);
 
 	/*
@@ -344,7 +337,6 @@ consider_new_or_clause(PlannerInfo *root, RelOptInfo *rel,
 		sjinfo.commute_below = NULL;
 		/* we don't bother trying to make the remaining fields valid */
 		sjinfo.lhs_strict = false;
-		sjinfo.delay_upper_joins = false;
 		sjinfo.semi_can_btree = false;
 		sjinfo.semi_can_hash = false;
 		sjinfo.semi_operators = NIL;
@@ -356,7 +348,7 @@ consider_new_or_clause(PlannerInfo *root, RelOptInfo *rel,
 
 		/* And hack cached selectivity so join size remains the same */
 		join_or_rinfo->norm_selec = orig_selec / or_selec;
-		/* ensure result stays in sane range, in particular not "redundant" */
+		/* ensure result stays in sane range */
 		if (join_or_rinfo->norm_selec > 1)
 			join_or_rinfo->norm_selec = 1;
 		/* as explained above, we don't touch outer_selec */
diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c
index af10dbd124f..9c6cb5eba7d 100644
--- a/src/backend/optimizer/util/placeholder.c
+++ b/src/backend/optimizer/util/placeholder.c
@@ -134,7 +134,6 @@ find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv)
 		phinfo->ph_eval_at = bms_copy(phv->phrels);
 		Assert(!bms_is_empty(phinfo->ph_eval_at));
 	}
-	/* ph_eval_at may change later, see update_placeholder_eval_levels */
 	phinfo->ph_needed = NULL;	/* initially it's unused */
 	/* for the moment, estimate width using just the datatype info */
 	phinfo->ph_width = get_typavgwidth(exprType((Node *) phv->phexpr),
@@ -285,102 +284,6 @@ find_placeholders_in_expr(PlannerInfo *root, Node *expr)
 }
 
 /*
- * update_placeholder_eval_levels
- *		Adjust the target evaluation levels for placeholders
- *
- * The initial eval_at level set by find_placeholder_info was the set of
- * rels used in the placeholder's expression (or the whole subselect below
- * the placeholder's syntactic location, if the expr is variable-free).
- * If the query contains any outer joins that can null any of those rels,
- * we must delay evaluation to above those joins.
- *
- * We repeat this operation each time we add another outer join to
- * root->join_info_list.  It's somewhat annoying to have to do that, but
- * since we don't have very much information on the placeholders' locations,
- * it's hard to avoid.  Each placeholder's eval_at level must be correct
- * by the time it starts to figure in outer-join delay decisions for higher
- * outer joins.
- *
- * In future we might want to put additional policy/heuristics here to
- * try to determine an optimal evaluation level.  The current rules will
- * result in evaluation at the lowest possible level.  However, pushing a
- * placeholder eval up the tree is likely to further constrain evaluation
- * order for outer joins, so it could easily be counterproductive; and we
- * don't have enough information at this point to make an intelligent choice.
- */
-void
-update_placeholder_eval_levels(PlannerInfo *root, SpecialJoinInfo *new_sjinfo)
-{
-	ListCell   *lc1;
-
-	foreach(lc1, root->placeholder_list)
-	{
-		PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(lc1);
-		Relids		syn_level = phinfo->ph_var->phrels;
-		Relids		eval_at;
-		bool		found_some;
-		ListCell   *lc2;
-
-		/*
-		 * We don't need to do any work on this placeholder unless the
-		 * newly-added outer join is syntactically beneath its location.
-		 */
-		if (!bms_is_subset(new_sjinfo->syn_lefthand, syn_level) ||
-			!bms_is_subset(new_sjinfo->syn_righthand, syn_level))
-			continue;
-
-		/*
-		 * Check for delays due to lower outer joins.  This is the same logic
-		 * as in check_outerjoin_delay in initsplan.c, except that we don't
-		 * have anything to do with the delay_upper_joins flags; delay of
-		 * upper outer joins will be handled later, based on the eval_at
-		 * values we compute now.
-		 */
-		eval_at = phinfo->ph_eval_at;
-
-		do
-		{
-			found_some = false;
-			foreach(lc2, root->join_info_list)
-			{
-				SpecialJoinInfo *sjinfo = (SpecialJoinInfo *) lfirst(lc2);
-
-				/* disregard joins not within the PHV's sub-select */
-				if (!bms_is_subset(sjinfo->syn_lefthand, syn_level) ||
-					!bms_is_subset(sjinfo->syn_righthand, syn_level))
-					continue;
-
-				/* do we reference any nullable rels of this OJ? */
-				if (bms_overlap(eval_at, sjinfo->min_righthand) ||
-					(sjinfo->jointype == JOIN_FULL &&
-					 bms_overlap(eval_at, sjinfo->min_lefthand)))
-				{
-					/* yes; have we included all its rels in eval_at? */
-					if (!bms_is_subset(sjinfo->min_lefthand, eval_at) ||
-						!bms_is_subset(sjinfo->min_righthand, eval_at))
-					{
-						/* no, so add them in */
-						eval_at = bms_add_members(eval_at,
-												  sjinfo->min_lefthand);
-						eval_at = bms_add_members(eval_at,
-												  sjinfo->min_righthand);
-						if (sjinfo->ojrelid)
-							eval_at = bms_add_member(eval_at, sjinfo->ojrelid);
-						/* we'll need another iteration */
-						found_some = true;
-					}
-				}
-			}
-		} while (found_some);
-
-		/* Can't move the PHV's eval_at level to above its syntactic level */
-		Assert(bms_is_subset(eval_at, syn_level));
-
-		phinfo->ph_eval_at = eval_at;
-	}
-}
-
-/*
  * fix_placeholder_input_needed_levels
  *		Adjust the "needed at" levels for placeholder inputs
  *
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index ebfb4ddd121..ad84cc43e11 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -284,6 +284,12 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent)
 		rel->top_parent_relids = rel->top_parent->relids;
 
 		/*
+		 * A child rel is below the same outer joins as its parent.  (We
+		 * presume this info was already calculated for the parent.)
+		 */
+		rel->nulling_relids = parent->nulling_relids;
+
+		/*
 		 * Also propagate lateral-reference information from appendrel parent
 		 * rels to their child rels.  We intentionally give each child rel the
 		 * same minimum parameterization, even though it's quite possible that
@@ -306,6 +312,7 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent)
 		rel->parent = NULL;
 		rel->top_parent = NULL;
 		rel->top_parent_relids = NULL;
+		rel->nulling_relids = NULL;
 		rel->direct_lateral_relids = NULL;
 		rel->lateral_relids = NULL;
 		rel->lateral_referencers = NULL;
@@ -685,6 +692,7 @@ build_join_rel(PlannerInfo *root,
 	joinrel->max_attr = 0;
 	joinrel->attr_needed = NULL;
 	joinrel->attr_widths = NULL;
+	joinrel->nulling_relids = NULL;
 	joinrel->lateral_vars = NIL;
 	joinrel->lateral_referencers = NULL;
 	joinrel->indexlist = NIL;
@@ -874,6 +882,7 @@ build_child_join_rel(PlannerInfo *root, RelOptInfo *outer_rel,
 	joinrel->max_attr = 0;
 	joinrel->attr_needed = NULL;
 	joinrel->attr_widths = NULL;
+	joinrel->nulling_relids = NULL;
 	joinrel->lateral_vars = NIL;
 	joinrel->lateral_referencers = NULL;
 	joinrel->indexlist = NIL;
@@ -1646,18 +1655,9 @@ get_joinrel_parampathinfo(PlannerInfo *root, RelOptInfo *joinrel,
 	{
 		RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
 
-		/*
-		 * In principle, join_clause_is_movable_into() should accept anything
-		 * returned by generate_join_implied_equalities(); but because its
-		 * analysis is only approximate, sometimes it doesn't.  So we
-		 * currently cannot use this Assert; instead just assume it's okay to
-		 * apply the joinclause at this level.
-		 */
-#ifdef NOT_USED
 		Assert(join_clause_is_movable_into(rinfo,
 										   joinrel->relids,
 										   join_and_req));
-#endif
 		if (join_clause_is_movable_into(rinfo,
 										outer_path->parent->relids,
 										outer_and_req))
@@ -1720,12 +1720,9 @@ get_joinrel_parampathinfo(PlannerInfo *root, RelOptInfo *joinrel,
 		{
 			RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
 
-			/* As above, can't quite assert this here */
-#ifdef NOT_USED
 			Assert(join_clause_is_movable_into(rinfo,
 											   outer_path->parent->relids,
 											   real_outer_and_req));
-#endif
 			if (!join_clause_is_movable_into(rinfo,
 											 outer_path->parent->relids,
 											 outer_and_req))
diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index 1350f011a62..c44bd2f8157 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -25,21 +25,17 @@ static RestrictInfo *make_restrictinfo_internal(PlannerInfo *root,
 												Expr *clause,
 												Expr *orclause,
 												bool is_pushed_down,
-												bool outerjoin_delayed,
 												bool pseudoconstant,
 												Index security_level,
 												Relids required_relids,
-												Relids outer_relids,
-												Relids nullable_relids);
+												Relids outer_relids);
 static Expr *make_sub_restrictinfos(PlannerInfo *root,
 									Expr *clause,
 									bool is_pushed_down,
-									bool outerjoin_delayed,
 									bool pseudoconstant,
 									Index security_level,
 									Relids required_relids,
-									Relids outer_relids,
-									Relids nullable_relids);
+									Relids outer_relids);
 
 
 /*
@@ -47,9 +43,9 @@ static Expr *make_sub_restrictinfos(PlannerInfo *root,
  *
  * Build a RestrictInfo node containing the given subexpression.
  *
- * The is_pushed_down, outerjoin_delayed, and pseudoconstant flags for the
+ * The is_pushed_down and pseudoconstant flags for the
  * RestrictInfo must be supplied by the caller, as well as the correct values
- * for security_level, outer_relids, and nullable_relids.
+ * for security_level and outer_relids.
  * required_relids can be NULL, in which case it defaults to the actual clause
  * contents (i.e., clause_relids).
  *
@@ -65,12 +61,10 @@ RestrictInfo *
 make_restrictinfo(PlannerInfo *root,
 				  Expr *clause,
 				  bool is_pushed_down,
-				  bool outerjoin_delayed,
 				  bool pseudoconstant,
 				  Index security_level,
 				  Relids required_relids,
-				  Relids outer_relids,
-				  Relids nullable_relids)
+				  Relids outer_relids)
 {
 	/*
 	 * If it's an OR clause, build a modified copy with RestrictInfos inserted
@@ -80,12 +74,10 @@ make_restrictinfo(PlannerInfo *root,
 		return (RestrictInfo *) make_sub_restrictinfos(root,
 													   clause,
 													   is_pushed_down,
-													   outerjoin_delayed,
 													   pseudoconstant,
 													   security_level,
 													   required_relids,
-													   outer_relids,
-													   nullable_relids);
+													   outer_relids);
 
 	/* Shouldn't be an AND clause, else AND/OR flattening messed up */
 	Assert(!is_andclause(clause));
@@ -94,12 +86,10 @@ make_restrictinfo(PlannerInfo *root,
 									  clause,
 									  NULL,
 									  is_pushed_down,
-									  outerjoin_delayed,
 									  pseudoconstant,
 									  security_level,
 									  required_relids,
-									  outer_relids,
-									  nullable_relids);
+									  outer_relids);
 }
 
 /*
@@ -112,12 +102,10 @@ make_restrictinfo_internal(PlannerInfo *root,
 						   Expr *clause,
 						   Expr *orclause,
 						   bool is_pushed_down,
-						   bool outerjoin_delayed,
 						   bool pseudoconstant,
 						   Index security_level,
 						   Relids required_relids,
-						   Relids outer_relids,
-						   Relids nullable_relids)
+						   Relids outer_relids)
 {
 	RestrictInfo *restrictinfo = makeNode(RestrictInfo);
 	Relids		baserels;
@@ -125,14 +113,12 @@ make_restrictinfo_internal(PlannerInfo *root,
 	restrictinfo->clause = clause;
 	restrictinfo->orclause = orclause;
 	restrictinfo->is_pushed_down = is_pushed_down;
-	restrictinfo->outerjoin_delayed = outerjoin_delayed;
 	restrictinfo->pseudoconstant = pseudoconstant;
 	restrictinfo->has_clone = false;	/* may get set by caller */
 	restrictinfo->is_clone = false; /* may get set by caller */
 	restrictinfo->can_join = false; /* may get set below */
 	restrictinfo->security_level = security_level;
 	restrictinfo->outer_relids = outer_relids;
-	restrictinfo->nullable_relids = nullable_relids;
 
 	/*
 	 * If it's potentially delayable by lower-level security quals, figure out
@@ -258,9 +244,9 @@ make_restrictinfo_internal(PlannerInfo *root,
  * implicit-AND lists at top level of RestrictInfo lists.  Only ORs and
  * simple clauses are valid RestrictInfos.
  *
- * The same is_pushed_down, outerjoin_delayed, and pseudoconstant flag
+ * The same is_pushed_down and pseudoconstant flag
  * values can be applied to all RestrictInfo nodes in the result.  Likewise
- * for security_level, outer_relids, and nullable_relids.
+ * for security_level and outer_relids.
  *
  * The given required_relids are attached to our top-level output,
  * but any OR-clause constituents are allowed to default to just the
@@ -270,12 +256,10 @@ static Expr *
 make_sub_restrictinfos(PlannerInfo *root,
 					   Expr *clause,
 					   bool is_pushed_down,
-					   bool outerjoin_delayed,
 					   bool pseudoconstant,
 					   Index security_level,
 					   Relids required_relids,
-					   Relids outer_relids,
-					   Relids nullable_relids)
+					   Relids outer_relids)
 {
 	if (is_orclause(clause))
 	{
@@ -287,22 +271,18 @@ make_sub_restrictinfos(PlannerInfo *root,
 							 make_sub_restrictinfos(root,
 													lfirst(temp),
 													is_pushed_down,
-													outerjoin_delayed,
 													pseudoconstant,
 													security_level,
 													NULL,
-													outer_relids,
-													nullable_relids));
+													outer_relids));
 		return (Expr *) make_restrictinfo_internal(root,
 												   clause,
 												   make_orclause(orlist),
 												   is_pushed_down,
-												   outerjoin_delayed,
 												   pseudoconstant,
 												   security_level,
 												   required_relids,
-												   outer_relids,
-												   nullable_relids);
+												   outer_relids);
 	}
 	else if (is_andclause(clause))
 	{
@@ -314,12 +294,10 @@ make_sub_restrictinfos(PlannerInfo *root,
 							  make_sub_restrictinfos(root,
 													 lfirst(temp),
 													 is_pushed_down,
-													 outerjoin_delayed,
 													 pseudoconstant,
 													 security_level,
 													 required_relids,
-													 outer_relids,
-													 nullable_relids));
+													 outer_relids));
 		return make_andclause(andlist);
 	}
 	else
@@ -327,12 +305,10 @@ make_sub_restrictinfos(PlannerInfo *root,
 												   clause,
 												   NULL,
 												   is_pushed_down,
-												   outerjoin_delayed,
 												   pseudoconstant,
 												   security_level,
 												   required_relids,
-												   outer_relids,
-												   nullable_relids);
+												   outer_relids);
 }
 
 /*
@@ -437,6 +413,21 @@ restriction_is_securely_promotable(RestrictInfo *restrictinfo,
 }
 
 /*
+ * Detect whether a RestrictInfo's clause is constant TRUE (note that it's
+ * surely of type boolean).  No such WHERE clause could survive qual
+ * canonicalization, but equivclass.c may generate such RestrictInfos for
+ * reasons discussed therein.  We should drop them again when creating
+ * the finished plan, which is handled by the next few functions.
+ */
+static inline bool
+rinfo_is_constant_true(RestrictInfo *rinfo)
+{
+	return IsA(rinfo->clause, Const) &&
+		!((Const *) rinfo->clause)->constisnull &&
+		DatumGetBool(((Const *) rinfo->clause)->constvalue);
+}
+
+/*
  * get_actual_clauses
  *
  * Returns a list containing the bare clauses from 'restrictinfo_list'.
@@ -455,6 +446,7 @@ get_actual_clauses(List *restrictinfo_list)
 		RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);
 
 		Assert(!rinfo->pseudoconstant);
+		Assert(!rinfo_is_constant_true(rinfo));
 
 		result = lappend(result, rinfo->clause);
 	}
@@ -466,6 +458,7 @@ get_actual_clauses(List *restrictinfo_list)
  *
  * Extract bare clauses from 'restrictinfo_list', returning either the
  * regular ones or the pseudoconstant ones per 'pseudoconstant'.
+ * Constant-TRUE clauses are dropped in any case.
  */
 List *
 extract_actual_clauses(List *restrictinfo_list,
@@ -478,7 +471,8 @@ extract_actual_clauses(List *restrictinfo_list,
 	{
 		RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);
 
-		if (rinfo->pseudoconstant == pseudoconstant)
+		if (rinfo->pseudoconstant == pseudoconstant &&
+			!rinfo_is_constant_true(rinfo))
 			result = lappend(result, rinfo->clause);
 	}
 	return result;
@@ -489,7 +483,7 @@ extract_actual_clauses(List *restrictinfo_list,
  *
  * Extract bare clauses from 'restrictinfo_list', separating those that
  * semantically match the join level from those that were pushed down.
- * Pseudoconstant clauses are excluded from the results.
+ * Pseudoconstant and constant-TRUE clauses are excluded from the results.
  *
  * This is only used at outer joins, since for plain joins we don't care
  * about pushed-down-ness.
@@ -511,13 +505,15 @@ extract_actual_join_clauses(List *restrictinfo_list,
 
 		if (RINFO_IS_PUSHED_DOWN(rinfo, joinrelids))
 		{
-			if (!rinfo->pseudoconstant)
+			if (!rinfo->pseudoconstant &&
+				!rinfo_is_constant_true(rinfo))
 				*otherquals = lappend(*otherquals, rinfo->clause);
 		}
 		else
 		{
 			/* joinquals shouldn't have been marked pseudoconstant */
 			Assert(!rinfo->pseudoconstant);
+			Assert(!rinfo_is_constant_true(rinfo));
 			*joinquals = lappend(*joinquals, rinfo->clause);
 		}
 	}
@@ -618,8 +614,17 @@ join_clause_is_movable_to(RestrictInfo *rinfo, RelOptInfo *baserel)
 	if (bms_is_member(baserel->relid, rinfo->outer_relids))
 		return false;
 
-	/* Target rel must not be nullable below the clause */
-	if (bms_is_member(baserel->relid, rinfo->nullable_relids))
+	/*
+	 * Target rel's Vars must not be nulled by any outer join.  We can check
+	 * this without groveling through the individual Vars by seeing whether
+	 * clause_relids (which includes all such Vars' varnullingrels) includes
+	 * any outer join that can null the target rel.  You might object that
+	 * this could reject the clause on the basis of an OJ relid that came from
+	 * some other rel's Var.  However, that would still mean that the clause
+	 * came from above that outer join and shouldn't be pushed down; so there
+	 * should be no false positives.
+	 */
+	if (bms_overlap(rinfo->clause_relids, baserel->nulling_relids))
 		return false;
 
 	/* Clause must not use any rels with LATERAL references to this rel */
@@ -651,18 +656,15 @@ join_clause_is_movable_to(RestrictInfo *rinfo, RelOptInfo *baserel)
  * relation plus the outer rels.  We also check that it does reference at
  * least one current Var, ensuring that the clause will be pushed down to
  * a unique place in a parameterized join tree.  And we check that we're
- * not pushing the clause into its outer-join outer side, nor down into
- * a lower outer join's inner side.
- *
- * The check about pushing a clause down into a lower outer join's inner side
- * is only approximate; it sometimes returns "false" when actually it would
- * be safe to use the clause here because we're still above the outer join
- * in question.  This is okay as long as the answers at different join levels
- * are consistent: it just means we might sometimes fail to push a clause as
- * far down as it could safely be pushed.  It's unclear whether it would be
- * worthwhile to do this more precisely.  (But if it's ever fixed to be
- * exactly accurate, there's an Assert in get_joinrel_parampathinfo() that
- * should be re-enabled.)
+ * not pushing the clause into its outer-join outer side.
+ *
+ * We used to need to check that we're not pushing the clause into a lower
+ * outer join's inner side.  However, now that clause_relids includes
+ * references to potentially-nulling outer joins, the other tests handle that
+ * concern.  If the clause references any Var coming from the inside of a
+ * lower outer join, its clause_relids will mention that outer join, causing
+ * the evaluability check to fail; while if it references no such Vars, the
+ * references-a-target-rel check will fail.
  *
  * There's no check here equivalent to join_clause_is_movable_to's test on
  * lateral_referencers.  We assume the caller wouldn't be inquiring unless
@@ -704,14 +706,5 @@ join_clause_is_movable_into(RestrictInfo *rinfo,
 	if (bms_overlap(currentrelids, rinfo->outer_relids))
 		return false;
 
-	/*
-	 * Target rel(s) must not be nullable below the clause.  This is
-	 * approximate, in the safe direction, because the current join might be
-	 * above the join where the nulling would happen, in which case the clause
-	 * would work correctly here.  But we don't have enough info to be sure.
-	 */
-	if (bms_overlap(currentrelids, rinfo->nullable_relids))
-		return false;
-
 	return true;
 }