Don't use bms_membership() in cases where we don't need to

00b41463c adjusted Bitmapset so that an empty set is always represented
as NULL.  This makes checking for empty sets far cheaper than it used
to be.

There were various places in the code where we'd call bms_membership()
to handle the 3 possible BMS_Membership values.  For the BMS_SINGLETON
case, we'd also call bms_singleton_member() to find the single set member.
This can now be done in a more optimal way by first checking if the set is
NULL and then not bothering with bms_membership() and simply call
bms_get_singleton_member() instead to find the single member.  This
function will return false if there are multiple members in the set.

Here we also tidy up some logic in examine_variable() for the single
member case.  There's now no need to call bms_is_member() as we've
already established that we're working with a singleton Bitmapset, so we
can just check if varRelid matches the singleton member.

Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/CAApHDvqW+CxNPcY245GaWiuqkkqgTudtG2ncGvvSjGn2wdTZLA@mail.gmail.com

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index b31d8921211e0e93a00bd755abc869f03f66a5f0..8295e7753db8cdc92ccc358a8c10fd0ef875fae4 100644 (file)
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -2634,15 +2634,17 @@ distribute_restrictinfo_to_rels(PlannerInfo *root,
    Relids      relids = restrictinfo->required_relids;
    RelOptInfo *rel;
 
-   switch (bms_membership(relids))
+   if (!bms_is_empty(relids))
    {
-       case BMS_SINGLETON:
+       int         relid;
 
+       if (bms_get_singleton_member(relids, &relid))
+       {
            /*
             * There is only one relation participating in the clause, so it
             * is a restriction clause for that relation.
             */
-           rel = find_base_rel(root, bms_singleton_member(relids));
+           rel = find_base_rel(root, relid);
 
            /* Add clause to rel's restriction list */
            rel->baserestrictinfo = lappend(rel->baserestrictinfo,
@@ -2650,9 +2652,9 @@ distribute_restrictinfo_to_rels(PlannerInfo *root,
            /* Update security level info */
            rel->baserestrict_min_security = Min(rel->baserestrict_min_security,
                                                 restrictinfo->security_level);
-           break;
-       case BMS_MULTIPLE:
-
+       }
+       else
+       {
            /*
             * The clause is a join clause, since there is more than one rel
             * in its relid set.
@@ -2675,15 +2677,15 @@ distribute_restrictinfo_to_rels(PlannerInfo *root,
             * Add clause to the join lists of all the relevant relations.
             */
            add_join_clause_to_rels(root, restrictinfo, relids);
-           break;
-       default:
-
-           /*
-            * clause references no rels, and therefore we have no place to
-            * attach it.  Shouldn't get here if callers are working properly.
-            */
-           elog(ERROR, "cannot cope with variable-free clause");
-           break;
+       }
+   }
+   else
+   {
+       /*
+        * clause references no rels, and therefore we have no place to attach
+        * it.  Shouldn't get here if callers are working properly.
+        */
+       elog(ERROR, "cannot cope with variable-free clause");
    }
 }
 

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 35c9e3c86fed1b9d2ae76b86a171dcfc8efe062d..e11d022827a05b5c58434dc9f6340acc9bafaa84 100644 (file)
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -5028,22 +5028,27 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
 
    onerel = NULL;
 
-   switch (bms_membership(varnos))
+   if (bms_is_empty(varnos))
    {
-       case BMS_EMPTY_SET:
-           /* No Vars at all ... must be pseudo-constant clause */
-           break;
-       case BMS_SINGLETON:
-           if (varRelid == 0 || bms_is_member(varRelid, varnos))
+       /* No Vars at all ... must be pseudo-constant clause */
+   }
+   else
+   {
+       int         relid;
+
+       if (bms_get_singleton_member(varnos, &relid))
+       {
+           if (varRelid == 0 || varRelid == relid)
            {
-               onerel = find_base_rel(root,
-                                      (varRelid ? varRelid : bms_singleton_member(varnos)));
+               onerel = find_base_rel(root, relid);
                vardata->rel = onerel;
                node = basenode;    /* strip any relabeling */
            }
            /* else treat it as a constant */
-           break;
-       case BMS_MULTIPLE:
+       }
+       else
+       {
+           /* varnos has multiple relids */
            if (varRelid == 0)
            {
                /* treat it as a variable of a join relation */
@@ -5058,7 +5063,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
                /* note: no point in expressional-index search here */
            }
            /* else treat it as a constant */
-           break;
+       }
    }
 
    bms_free(varnos);
@@ -6381,17 +6386,14 @@ find_join_input_rel(PlannerInfo *root, Relids relids)
 {
    RelOptInfo *rel = NULL;
 
-   switch (bms_membership(relids))
+   if (!bms_is_empty(relids))
    {
-       case BMS_EMPTY_SET:
-           /* should not happen */
-           break;
-       case BMS_SINGLETON:
-           rel = find_base_rel(root, bms_singleton_member(relids));
-           break;
-       case BMS_MULTIPLE:
+       int         relid;
+
+       if (bms_get_singleton_member(relids, &relid))
+           rel = find_base_rel(root, relid);
+       else
            rel = find_join_rel(root, relids);
-           break;
    }
 
    if (rel == NULL)