Remove local optimizations of empty Bitmapsets into null pointers.

tglsfdc · pull[bot] · commit 4dec165a2f61 · 2023-10-18T06:13:16.000Z
These are all dead code now that it's done centrally. Patch by me; thanks to Nathan Bossart and Richard Guo for review. Discussion: https://postgr.es/m/1159933.1677621588@sss.pgh.pa.us
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
@@ -877,15 +877,7 @@ UpdateChangedParamSet(PlanState *node, Bitmapset *newchg)
 	 * include anything else into its chgParam set.
 	 */
 	parmset = bms_intersect(node->plan->allParam, newchg);
-
-	/*
-	 * Keep node->chgParam == NULL if there's not actually any members; this
-	 * allows the simplest possible tests in executor node files.
-	 */
-	if (!bms_is_empty(parmset))
-		node->chgParam = bms_join(node->chgParam, parmset);
-	else
-		bms_free(parmset);
+	node->chgParam = bms_join(node->chgParam, parmset);
 }
 
 /*
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
@@ -949,9 +949,6 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
 
 	/* We do not want the index's rel itself listed in outer_relids */
 	outer_relids = bms_del_member(outer_relids, rel->relid);
-	/* Enforce convention that outer_relids is exactly NULL if empty */
-	if (bms_is_empty(outer_relids))
-		outer_relids = NULL;
 
 	/* Compute loop_count for cost estimation purposes */
 	loop_count = get_loop_count(root, rel->relid, outer_relids);
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
@@ -679,16 +679,10 @@ create_lateral_join_info(PlannerInfo *root)
 
 		/* Nothing to do at rels with no lateral refs */
 		lateral_relids = brel->lateral_relids;
-		if (lateral_relids == NULL)
+		if (bms_is_empty(lateral_relids))
 			continue;
 
-		/*
-		 * We should not have broken the invariant that lateral_relids is
-		 * exactly NULL if empty.
-		 */
-		Assert(!bms_is_empty(lateral_relids));
-
-		/* Also, no rel should have a lateral dependency on itself */
+		/* No rel should have a lateral dependency on itself */
 		Assert(!bms_is_member(rti, lateral_relids));
 
 		/* Mark this rel's referencees */
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
@@ -2825,15 +2825,6 @@ finalize_plan(PlannerInfo *root, Plan *plan,
 	/* but not any initplan setParams */
 	plan->extParam = bms_del_members(plan->extParam, initSetParam);
 
-	/*
-	 * For speed at execution time, make sure extParam/allParam are actually
-	 * NULL if they are empty sets.
-	 */
-	if (bms_is_empty(plan->extParam))
-		plan->extParam = NULL;
-	if (bms_is_empty(plan->allParam))
-		plan->allParam = NULL;
-
 	return plan->allParam;
 }
 
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
@@ -2372,12 +2372,6 @@ calc_nestloop_required_outer(Relids outerrelids,
 	/* ... and remove any mention of now-satisfied outer rels */
 	required_outer = bms_del_members(required_outer,
 									 outerrelids);
-	/* maintain invariant that required_outer is exactly NULL if empty */
-	if (bms_is_empty(required_outer))
-	{
-		bms_free(required_outer);
-		required_outer = NULL;
-	}
 	return required_outer;
 }
 
diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c
@@ -125,8 +125,6 @@ find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv)
 	 */
 	rels_used = pull_varnos(root, (Node *) phv->phexpr);
 	phinfo->ph_lateral = bms_difference(rels_used, phv->phrels);
-	if (bms_is_empty(phinfo->ph_lateral))
-		phinfo->ph_lateral = NULL;	/* make it exactly NULL if empty */
 	phinfo->ph_eval_at = bms_int_members(rels_used, phv->phrels);
 	/* If no contained vars, force evaluation at syntactic location */
 	if (bms_is_empty(phinfo->ph_eval_at))
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
@@ -772,8 +772,6 @@ build_join_rel(PlannerInfo *root,
 	 */
 	joinrel->direct_lateral_relids =
 		bms_del_members(joinrel->direct_lateral_relids, joinrel->relids);
-	if (bms_is_empty(joinrel->direct_lateral_relids))
-		joinrel->direct_lateral_relids = NULL;
 
 	/*
 	 * Construct restrict and join clause lists for the new joinrel. (The
@@ -1024,11 +1022,6 @@ min_join_parameterization(PlannerInfo *root,
 	 */
 	result = bms_union(outer_rel->lateral_relids, inner_rel->lateral_relids);
 	result = bms_del_members(result, joinrelids);
-
-	/* Maintain invariant that result is exactly NULL if empty */
-	if (bms_is_empty(result))
-		result = NULL;
-
 	return result;
 }
 
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
@@ -1247,16 +1247,11 @@ remove_nulling_relids_mutator(Node *node,
 			!bms_is_member(var->varno, context->except_relids) &&
 			bms_overlap(var->varnullingrels, context->removable_relids))
 		{
-			Relids		newnullingrels = bms_difference(var->varnullingrels,
-														context->removable_relids);
-
-			/* Micro-optimization: ensure nullingrels is NULL if empty */
-			if (bms_is_empty(newnullingrels))
-				newnullingrels = NULL;
 			/* Copy the Var ... */
 			var = copyObject(var);
 			/* ... and replace the copy's varnullingrels field */
-			var->varnullingrels = newnullingrels;
+			var->varnullingrels = bms_difference(var->varnullingrels,
+												 context->removable_relids);
 			return (Node *) var;
 		}
 		/* Otherwise fall through to copy the Var normally */
@@ -1268,26 +1263,20 @@ remove_nulling_relids_mutator(Node *node,
 		if (phv->phlevelsup == context->sublevels_up &&
 			!bms_overlap(phv->phrels, context->except_relids))
 		{
-			Relids		newnullingrels = bms_difference(phv->phnullingrels,
-														context->removable_relids);
-
 			/*
-			 * Micro-optimization: ensure nullingrels is NULL if empty.
-			 *
 			 * Note: it might seem desirable to remove the PHV altogether if
 			 * phnullingrels goes to empty.  Currently we dare not do that
 			 * because we use PHVs in some cases to enforce separate identity
 			 * of subexpressions; see wrap_non_vars usages in prepjointree.c.
 			 */
-			if (bms_is_empty(newnullingrels))
-				newnullingrels = NULL;
 			/* Copy the PlaceHolderVar and mutate what's below ... */
 			phv = (PlaceHolderVar *)
 				expression_tree_mutator(node,
 										remove_nulling_relids_mutator,
 										(void *) context);
 			/* ... and replace the copy's phnullingrels field */
-			phv->phnullingrels = newnullingrels;
+			phv->phnullingrels = bms_difference(phv->phnullingrels,
+												context->removable_relids);
 			/* We must also update phrels, if it contains a removable RTI */
 			phv->phrels = bms_difference(phv->phrels,
 										 context->removable_relids);
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
@@ -6240,12 +6240,9 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
 	Assert(expr->plan != NULL);
 
 	/*
-	 * We only need a ParamListInfo if the expression has parameters.  In
-	 * principle we should test with bms_is_empty(), but we use a not-null
-	 * test because it's faster.  In current usage bits are never removed from
-	 * expr->paramnos, only added, so this test is correct anyway.
+	 * We only need a ParamListInfo if the expression has parameters.
 	 */
-	if (expr->paramnos)
+	if (!bms_is_empty(expr->paramnos))
 	{
 		/* Use the common ParamListInfo */
 		paramLI = estate->paramLI;
