From ae52bce53b203fdee7427963613bf00e42dcddea Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 1 Mar 2023 17:52:25 -0500 Subject: [PATCH v2 4/4] Remove local optimizations of empty Bitmapsets into null pointers. These are all dead code now that it's done centrally. Discussion: https://postgr.es/m/1159933.1677621588@sss.pgh.pa.us --- src/backend/executor/execUtils.c | 10 +--------- src/backend/optimizer/path/indxpath.c | 3 --- src/backend/optimizer/plan/subselect.c | 9 --------- src/backend/optimizer/util/pathnode.c | 6 ------ src/backend/optimizer/util/placeholder.c | 2 -- src/backend/optimizer/util/relnode.c | 7 ------- src/backend/rewrite/rewriteManip.c | 19 ++++--------------- src/pl/plpgsql/src/pl_exec.c | 7 ++----- 8 files changed, 7 insertions(+), 56 deletions(-) diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index c33a3c0bec..cfa95a07e4 100644 --- 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 index 011a0337da..9f4698f2a2 100644 --- 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/subselect.c b/src/backend/optimizer/plan/subselect.c index 22ffe4ca42..052263aea6 100644 --- 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 index d749b50578..e8e06397a9 100644 --- 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 index 9c6cb5eba7..b1723578e6 100644 --- 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 index 9ad44a0508..68fd033595 100644 --- 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 index 04718f66c0..d28d0da621 100644 --- 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 index ffd6d2e3bc..b0a2cac227 100644 --- 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; -- 2.31.1