Add bms_next_member(), and use it where appropriate.
authorTom Lane <[email protected]>
Fri, 28 Nov 2014 18:37:25 +0000 (13:37 -0500)
committerTom Lane <[email protected]>
Fri, 28 Nov 2014 18:37:25 +0000 (13:37 -0500)
This patch adds a way of iterating through the members of a bitmapset
nondestructively, unlike the old way with bms_first_member().  While
bms_next_member() is very slightly slower than bms_first_member()
(at least for typical-size bitmapsets), eliminating the need to palloc
and pfree a temporary copy of the target bitmapset is a significant win.
So this method should be preferred in all cases where a temporary copy
would be necessary.

Tom Lane, with suggestions from Dean Rasheed and David Rowley

13 files changed:
contrib/postgres_fdw/postgres_fdw.c
contrib/sepgsql/dml.c
src/backend/executor/execMain.c
src/backend/nodes/bitmapset.c
src/backend/nodes/outfuncs.c
src/backend/optimizer/path/allpaths.c
src/backend/optimizer/path/indxpath.c
src/backend/optimizer/util/joininfo.c
src/backend/optimizer/util/var.c
src/backend/rewrite/rewriteHandler.c
src/backend/rewrite/rewriteManip.c
src/include/nodes/bitmapset.h
src/pl/plpgsql/src/pl_exec.c

index 76bda73935779093f7174f356a63ee6fb70f6b74..c3039a6480e0d8f4d98f8595ac968002efb2a415 100644 (file)
@@ -1198,15 +1198,17 @@ postgresPlanForeignModify(PlannerInfo *root,
    }
    else if (operation == CMD_UPDATE)
    {
-       Bitmapset  *tmpset = bms_copy(rte->modifiedCols);
-       AttrNumber  col;
+       int         col;
 
-       while ((col = bms_first_member(tmpset)) >= 0)
+       col = -1;
+       while ((col = bms_next_member(rte->modifiedCols, col)) >= 0)
        {
-           col += FirstLowInvalidHeapAttributeNumber;
-           if (col <= InvalidAttrNumber)       /* shouldn't happen */
+           /* bit numbers are offset by FirstLowInvalidHeapAttributeNumber */
+           AttrNumber  attno = col + FirstLowInvalidHeapAttributeNumber;
+
+           if (attno <= InvalidAttrNumber)     /* shouldn't happen */
                elog(ERROR, "system-column update is not supported");
-           targetAttrs = lappend_int(targetAttrs, col);
+           targetAttrs = lappend_int(targetAttrs, attno);
        }
    }
 
index bb82c0d6d2a846e5bc76e0a7aff477102ef7f7bc..9e9fa047461db999565fb9e978678aa056d66b20 100644 (file)
@@ -93,10 +93,7 @@ fixup_whole_row_references(Oid relOid, Bitmapset *columns)
 static Bitmapset *
 fixup_inherited_columns(Oid parentId, Oid childId, Bitmapset *columns)
 {
-   AttrNumber  attno;
-   Bitmapset  *tmpset;
    Bitmapset  *result = NULL;
-   char       *attname;
    int         index;
 
    /*
@@ -105,10 +102,12 @@ fixup_inherited_columns(Oid parentId, Oid childId, Bitmapset *columns)
    if (parentId == childId)
        return columns;
 
-   tmpset = bms_copy(columns);
-   while ((index = bms_first_member(tmpset)) > 0)
+   index = -1;
+   while ((index = bms_next_member(columns, index)) >= 0)
    {
-       attno = index + FirstLowInvalidHeapAttributeNumber;
+       /* bit numbers are offset by FirstLowInvalidHeapAttributeNumber */
+       AttrNumber  attno = index + FirstLowInvalidHeapAttributeNumber;
+       char       *attname;
 
        /*
         * whole-row-reference shall be fixed-up later
@@ -128,12 +127,11 @@ fixup_inherited_columns(Oid parentId, Oid childId, Bitmapset *columns)
            elog(ERROR, "cache lookup failed for attribute %s of relation %u",
                 attname, childId);
 
-       index = attno - FirstLowInvalidHeapAttributeNumber;
-       result = bms_add_member(result, index);
+       result = bms_add_member(result,
+                               attno - FirstLowInvalidHeapAttributeNumber);
 
        pfree(attname);
    }
-   bms_free(tmpset);
 
    return result;
 }
index c499486f01652bf25fff0b47a2bb4611fd8ff5c0..8c799d3c21b480e38bf7ba60a88650d7493cf4fb 100644 (file)
@@ -547,7 +547,6 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
    AclMode     remainingPerms;
    Oid         relOid;
    Oid         userid;
-   Bitmapset  *tmpset;
    int         col;
 
    /*
@@ -614,12 +613,13 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
                    return false;
            }
 
-           tmpset = bms_copy(rte->selectedCols);
-           while ((col = bms_first_member(tmpset)) >= 0)
+           col = -1;
+           while ((col = bms_next_member(rte->selectedCols, col)) >= 0)
            {
-               /* remove the column number offset */
-               col += FirstLowInvalidHeapAttributeNumber;
-               if (col == InvalidAttrNumber)
+               /* bit #s are offset by FirstLowInvalidHeapAttributeNumber */
+               AttrNumber  attno = col + FirstLowInvalidHeapAttributeNumber;
+
+               if (attno == InvalidAttrNumber)
                {
                    /* Whole-row reference, must have priv on all cols */
                    if (pg_attribute_aclcheck_all(relOid, userid, ACL_SELECT,
@@ -628,12 +628,11 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
                }
                else
                {
-                   if (pg_attribute_aclcheck(relOid, col, userid,
+                   if (pg_attribute_aclcheck(relOid, attno, userid,
                                              ACL_SELECT) != ACLCHECK_OK)
                        return false;
                }
            }
-           bms_free(tmpset);
        }
 
        /*
@@ -656,24 +655,24 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
                    return false;
            }
 
-           tmpset = bms_copy(rte->modifiedCols);
-           while ((col = bms_first_member(tmpset)) >= 0)
+           col = -1;
+           while ((col = bms_next_member(rte->modifiedCols, col)) >= 0)
            {
-               /* remove the column number offset */
-               col += FirstLowInvalidHeapAttributeNumber;
-               if (col == InvalidAttrNumber)
+               /* bit #s are offset by FirstLowInvalidHeapAttributeNumber */
+               AttrNumber  attno = col + FirstLowInvalidHeapAttributeNumber;
+
+               if (attno == InvalidAttrNumber)
                {
                    /* whole-row reference can't happen here */
                    elog(ERROR, "whole-row update is not implemented");
                }
                else
                {
-                   if (pg_attribute_aclcheck(relOid, col, userid,
+                   if (pg_attribute_aclcheck(relOid, attno, userid,
                                              remainingPerms) != ACLCHECK_OK)
                        return false;
                }
            }
-           bms_free(tmpset);
        }
    }
    return true;
index c927b7891f526fdaed5878818ac3a276709f2003..26a0f872b3b9297d9dab75d3a5bf990f914f2fd8 100644 (file)
@@ -793,7 +793,7 @@ bms_join(Bitmapset *a, Bitmapset *b)
    return result;
 }
 
-/*----------
+/*
  * bms_first_member - find and remove first member of a set
  *
  * Returns -1 if set is empty.  NB: set is destructively modified!
@@ -801,11 +801,11 @@ bms_join(Bitmapset *a, Bitmapset *b)
  * This is intended as support for iterating through the members of a set.
  * The typical pattern is
  *
- *         tmpset = bms_copy(inputset);
- *         while ((x = bms_first_member(tmpset)) >= 0)
+ *         while ((x = bms_first_member(inputset)) >= 0)
  *             process member x;
- *         bms_free(tmpset);
- *----------
+ *
+ * CAUTION: this destroys the content of "inputset".  If the set must
+ * not be modified, use bms_next_member instead.
  */
 int
 bms_first_member(Bitmapset *a)
@@ -840,6 +840,64 @@ bms_first_member(Bitmapset *a)
    return -1;
 }
 
+/*
+ * bms_next_member - find next member of a set
+ *
+ * Returns smallest member greater than "prevbit", or -2 if there is none.
+ * "prevbit" must NOT be less than -1, or the behavior is unpredictable.
+ *
+ * This is intended as support for iterating through the members of a set.
+ * The typical pattern is
+ *
+ *         x = -1;
+ *         while ((x = bms_next_member(inputset, x)) >= 0)
+ *             process member x;
+ *
+ * Notice that when there are no more members, we return -2, not -1 as you
+ * might expect.  The rationale for that is to allow distinguishing the
+ * loop-not-started state (x == -1) from the loop-completed state (x == -2).
+ * It makes no difference in simple loop usage, but complex iteration logic
+ * might need such an ability.
+ */
+int
+bms_next_member(const Bitmapset *a, int prevbit)
+{
+   int         nwords;
+   int         wordnum;
+   bitmapword  mask;
+
+   if (a == NULL)
+       return -2;
+   nwords = a->nwords;
+   prevbit++;
+   mask = (~(bitmapword) 0) << BITNUM(prevbit);
+   for (wordnum = WORDNUM(prevbit); wordnum < nwords; wordnum++)
+   {
+       bitmapword  w = a->words[wordnum];
+
+       /* ignore bits before prevbit */
+       w &= mask;
+
+       if (w != 0)
+       {
+           int         result;
+
+           result = wordnum * BITS_PER_BITMAPWORD;
+           while ((w & 255) == 0)
+           {
+               w >>= 8;
+               result += 8;
+           }
+           result += rightmost_one_pos[w & 255];
+           return result;
+       }
+
+       /* in subsequent words, consider all bits */
+       mask = (~(bitmapword) 0);
+   }
+   return -2;
+}
+
 /*
  * bms_hash_value - compute a hash key for a Bitmapset
  *
index ca9bd4f7c7f2e145a9ce08696282fac90bd4964a..edbd09f9425db934a2b14058df247d0738b3544d 100644 (file)
@@ -184,15 +184,13 @@ _outList(StringInfo str, const List *node)
 static void
 _outBitmapset(StringInfo str, const Bitmapset *bms)
 {
-   Bitmapset  *tmpset;
    int         x;
 
    appendStringInfoChar(str, '(');
    appendStringInfoChar(str, 'b');
-   tmpset = bms_copy(bms);
-   while ((x = bms_first_member(tmpset)) >= 0)
+   x = -1;
+   while ((x = bms_next_member(bms, x)) >= 0)
        appendStringInfo(str, " %d", x);
-   bms_free(tmpset);
    appendStringInfoChar(str, ')');
 }
 
index 25f30676f0258e32bfa34378a73dbfab624754e2..449fdc3474264db4a0a1d3adda294beb810357db 100644 (file)
@@ -2272,19 +2272,17 @@ remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel)
 static void
 print_relids(Relids relids)
 {
-   Relids      tmprelids;
    int         x;
    bool        first = true;
 
-   tmprelids = bms_copy(relids);
-   while ((x = bms_first_member(tmprelids)) >= 0)
+   x = -1;
+   while ((x = bms_next_member(relids, x)) >= 0)
    {
        if (!first)
            printf(" ");
        printf("%d", x);
        first = false;
    }
-   bms_free(tmprelids);
 }
 
 static void
index 1ee3b93b1e3cc6cab6af828d1f770029764669a1..7aab644e4563b518e257c90f97db4bc70249f44a 100644 (file)
@@ -1875,9 +1875,8 @@ get_loop_count(PlannerInfo *root, Relids outer_relids)
    {
        int         relid;
 
-       /* Need a working copy since bms_first_member is destructive */
-       outer_relids = bms_copy(outer_relids);
-       while ((relid = bms_first_member(outer_relids)) >= 0)
+       relid = -1;
+       while ((relid = bms_next_member(outer_relids, relid)) >= 0)
        {
            RelOptInfo *outer_rel;
 
@@ -1900,7 +1899,6 @@ get_loop_count(PlannerInfo *root, Relids outer_relids)
            if (result == 1.0 || result > outer_rel->rows)
                result = outer_rel->rows;
        }
-       bms_free(outer_relids);
    }
    return result;
 }
index 0418946d7141b361d33f52a73bd7ca76ba7035a2..40af38d7ada23be604f564e746580329ce3528e7 100644 (file)
@@ -96,17 +96,15 @@ add_join_clause_to_rels(PlannerInfo *root,
                        RestrictInfo *restrictinfo,
                        Relids join_relids)
 {
-   Relids      tmprelids;
    int         cur_relid;
 
-   tmprelids = bms_copy(join_relids);
-   while ((cur_relid = bms_first_member(tmprelids)) >= 0)
+   cur_relid = -1;
+   while ((cur_relid = bms_next_member(join_relids, cur_relid)) >= 0)
    {
        RelOptInfo *rel = find_base_rel(root, cur_relid);
 
        rel->joininfo = lappend(rel->joininfo, restrictinfo);
    }
-   bms_free(tmprelids);
 }
 
 /*
@@ -125,11 +123,10 @@ remove_join_clause_from_rels(PlannerInfo *root,
                             RestrictInfo *restrictinfo,
                             Relids join_relids)
 {
-   Relids      tmprelids;
    int         cur_relid;
 
-   tmprelids = bms_copy(join_relids);
-   while ((cur_relid = bms_first_member(tmprelids)) >= 0)
+   cur_relid = -1;
+   while ((cur_relid = bms_next_member(join_relids, cur_relid)) >= 0)
    {
        RelOptInfo *rel = find_base_rel(root, cur_relid);
 
@@ -140,5 +137,4 @@ remove_join_clause_from_rels(PlannerInfo *root,
        Assert(list_member_ptr(rel->joininfo, restrictinfo));
        rel->joininfo = list_delete_ptr(rel->joininfo, restrictinfo);
    }
-   bms_free(tmprelids);
 }
index d4f46b8d4614999c163bec31fb2e78ae5377dcca..a64a8d7d5ffb408a959c9b51c0fac13f726fe6a6 100644 (file)
@@ -773,11 +773,10 @@ static Relids
 alias_relid_set(PlannerInfo *root, Relids relids)
 {
    Relids      result = NULL;
-   Relids      tmprelids;
    int         rtindex;
 
-   tmprelids = bms_copy(relids);
-   while ((rtindex = bms_first_member(tmprelids)) >= 0)
+   rtindex = -1;
+   while ((rtindex = bms_next_member(relids, rtindex)) >= 0)
    {
        RangeTblEntry *rte = rt_fetch(rtindex, root->parse->rtable);
 
@@ -786,6 +785,5 @@ alias_relid_set(PlannerInfo *root, Relids relids)
        else
            result = bms_add_member(result, rtindex);
    }
-   bms_free(tmprelids);
    return result;
 }
index ad983c7158b3ca7071be222fd661d777b0b90f99..2ed64279f07bec49ed7ee8f3fb23e265b7c912bb 100644 (file)
@@ -2456,11 +2456,10 @@ static Bitmapset *
 adjust_view_column_set(Bitmapset *cols, List *targetlist)
 {
    Bitmapset  *result = NULL;
-   Bitmapset  *tmpcols;
-   AttrNumber  col;
+   int         col;
 
-   tmpcols = bms_copy(cols);
-   while ((col = bms_first_member(tmpcols)) >= 0)
+   col = -1;
+   while ((col = bms_next_member(cols, col)) >= 0)
    {
        /* bit numbers are offset by FirstLowInvalidHeapAttributeNumber */
        AttrNumber  attno = col + FirstLowInvalidHeapAttributeNumber;
@@ -2510,7 +2509,6 @@ adjust_view_column_set(Bitmapset *cols, List *targetlist)
                     attno);
        }
    }
-   bms_free(tmpcols);
 
    return result;
 }
index fb203146b137f791510278d2154029dff521b5f5..c9e4b68341e2750fe1305b6ee71a6f7a2a50d830 100644 (file)
@@ -454,13 +454,11 @@ static Relids
 offset_relid_set(Relids relids, int offset)
 {
    Relids      result = NULL;
-   Relids      tmprelids;
    int         rtindex;
 
-   tmprelids = bms_copy(relids);
-   while ((rtindex = bms_first_member(tmprelids)) >= 0)
+   rtindex = -1;
+   while ((rtindex = bms_next_member(relids, rtindex)) >= 0)
        result = bms_add_member(result, rtindex + offset);
-   bms_free(tmprelids);
    return result;
 }
 
index f77060844f9596f7f8853d207eb66066d0fc3410..a78ff4886d61fca48d3d59444645c615bd7eaed8 100644 (file)
@@ -89,6 +89,7 @@ extern Bitmapset *bms_join(Bitmapset *a, Bitmapset *b);
 
 /* support for iterating through the integer elements of a set: */
 extern int bms_first_member(Bitmapset *a);
+extern int bms_next_member(const Bitmapset *a, int prevbit);
 
 /* support for hashtables using Bitmapsets as keys: */
 extern uint32 bms_hash_value(const Bitmapset *a);
index 11cb47b5225d987b815d652adc77247b63e2022c..d1feef78b712d748e08e0b89ce1a22d8622961c9 100644 (file)
@@ -5263,7 +5263,6 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
     */
    if (!bms_is_empty(expr->paramnos))
    {
-       Bitmapset  *tmpset;
        int         dno;
 
        paramLI = (ParamListInfo)
@@ -5276,8 +5275,8 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
        paramLI->numParams = estate->ndatums;
 
        /* Instantiate values for "safe" parameters of the expression */
-       tmpset = bms_copy(expr->paramnos);
-       while ((dno = bms_first_member(tmpset)) >= 0)
+       dno = -1;
+       while ((dno = bms_next_member(expr->paramnos, dno)) >= 0)
        {
            PLpgSQL_datum *datum = estate->datums[dno];
 
@@ -5292,7 +5291,6 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
                prm->ptype = var->datatype->typoid;
            }
        }
-       bms_free(tmpset);
 
        /*
         * Set up link to active expr where the hook functions can find it.
@@ -6528,15 +6526,14 @@ format_expr_params(PLpgSQL_execstate *estate,
    int         paramno;
    int         dno;
    StringInfoData paramstr;
-   Bitmapset  *tmpset;
 
    if (!expr->paramnos)
        return NULL;
 
    initStringInfo(&paramstr);
-   tmpset = bms_copy(expr->paramnos);
    paramno = 0;
-   while ((dno = bms_first_member(tmpset)) >= 0)
+   dno = -1;
+   while ((dno = bms_next_member(expr->paramnos, dno)) >= 0)
    {
        Datum       paramdatum;
        Oid         paramtypeid;
@@ -6572,7 +6569,6 @@ format_expr_params(PLpgSQL_execstate *estate,
 
        paramno++;
    }
-   bms_free(tmpset);
 
    return paramstr.data;
 }