Remove bms_first_member().

This function has been semi-deprecated ever since we invented
bms_next_member().  Its habit of scribbling on the input bitmapset
isn't great, plus for sufficiently large bitmapsets it would take
O(N^2) time to complete a loop.  Now we have the additional problem
that reducing the input to empty while leaving it still accessible
would violate a planned invariant.  So let's just get rid of it,
after updating the few extant callers to use bms_next_member().

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/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 8ccc1675488eeade58dc3847d33b9a29c1718332..2d2b0b6a6b91c24cad626259d9c902bbbb24dffa 100644 (file)
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -858,7 +858,7 @@ check_selective_binary_conversion(RelOptInfo *baserel,
    ListCell   *lc;
    Relation    rel;
    TupleDesc   tupleDesc;
-   AttrNumber  attnum;
+   int         attidx;
    Bitmapset  *attrs_used = NULL;
    bool        has_wholerow = false;
    int         numattrs;
@@ -901,10 +901,11 @@ check_selective_binary_conversion(RelOptInfo *baserel,
    rel = table_open(foreigntableid, AccessShareLock);
    tupleDesc = RelationGetDescr(rel);
 
-   while ((attnum = bms_first_member(attrs_used)) >= 0)
+   attidx = -1;
+   while ((attidx = bms_next_member(attrs_used, attidx)) >= 0)
    {
-       /* Adjust for system attributes. */
-       attnum += FirstLowInvalidHeapAttributeNumber;
+       /* attidx is zero-based, attnum is the normal attribute number */
+       AttrNumber  attnum = attidx + FirstLowInvalidHeapAttributeNumber;
 
        if (attnum == 0)
        {

diff --git a/contrib/sepgsql/dml.c b/contrib/sepgsql/dml.c
index 3cc927c79ed76c7bfcc5f6f1a3a4de209ea5794b..8c8f6f1e3a115b5b7926160e4ad46a9953350b8e 100644 (file)
--- a/contrib/sepgsql/dml.c
+++ b/contrib/sepgsql/dml.c
@@ -231,7 +231,8 @@ check_relation_privileges(Oid relOid,
    updated = fixup_whole_row_references(relOid, updated);
    columns = bms_union(selected, bms_union(inserted, updated));
 
-   while ((index = bms_first_member(columns)) >= 0)
+   index = -1;
+   while ((index = bms_next_member(columns, index)) >= 0)
    {
        AttrNumber  attnum;
        uint32      column_perms = 0;

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7eb79cee58d6b26411dd6c388641242d63f33682..4f50e0dd3478d89defe4a2d18ce8bc3d2e142c2c 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3859,9 +3859,6 @@ heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2,
  * has_external indicates if any of the unmodified attributes (from those
  * listed as interesting) of the old tuple is a member of external_cols and is
  * stored externally.
- *
- * The input interesting_cols bitmapset is destructively modified; that is OK
- * since this is invoked at most once in heap_update.
  */
 static Bitmapset *
 HeapDetermineColumnsInfo(Relation relation,
@@ -3870,19 +3867,20 @@ HeapDetermineColumnsInfo(Relation relation,
                         HeapTuple oldtup, HeapTuple newtup,
                         bool *has_external)
 {
-   int         attrnum;
+   int         attidx;
    Bitmapset  *modified = NULL;
    TupleDesc   tupdesc = RelationGetDescr(relation);
 
-   while ((attrnum = bms_first_member(interesting_cols)) >= 0)
+   attidx = -1;
+   while ((attidx = bms_next_member(interesting_cols, attidx)) >= 0)
    {
+       /* attidx is zero-based, attrnum is the normal attribute number */
+       AttrNumber  attrnum = attidx + FirstLowInvalidHeapAttributeNumber;
        Datum       value1,
                    value2;
        bool        isnull1,
                    isnull2;
 
-       attrnum += FirstLowInvalidHeapAttributeNumber;
-
        /*
         * If it's a whole-tuple reference, say "not equal".  It's not really
         * worth supporting this case, since it could only succeed after a
@@ -3890,9 +3888,7 @@ HeapDetermineColumnsInfo(Relation relation,
         */
        if (attrnum == 0)
        {
-           modified = bms_add_member(modified,
-                                     attrnum -
-                                     FirstLowInvalidHeapAttributeNumber);
+           modified = bms_add_member(modified, attidx);
            continue;
        }
 
@@ -3905,9 +3901,7 @@ HeapDetermineColumnsInfo(Relation relation,
        {
            if (attrnum != TableOidAttributeNumber)
            {
-               modified = bms_add_member(modified,
-                                         attrnum -
-                                         FirstLowInvalidHeapAttributeNumber);
+               modified = bms_add_member(modified, attidx);
                continue;
            }
        }
@@ -3924,9 +3918,7 @@ HeapDetermineColumnsInfo(Relation relation,
        if (!heap_attr_equals(tupdesc, attrnum, value1,
                              value2, isnull1, isnull2))
        {
-           modified = bms_add_member(modified,
-                                     attrnum -
-                                     FirstLowInvalidHeapAttributeNumber);
+           modified = bms_add_member(modified, attidx);
            continue;
        }
 
@@ -3943,8 +3935,7 @@ HeapDetermineColumnsInfo(Relation relation,
         * member of external_cols.
         */
        if (VARATT_IS_EXTERNAL((struct varlena *) DatumGetPointer(value1)) &&
-           bms_is_member(attrnum - FirstLowInvalidHeapAttributeNumber,
-                         external_cols))
+           bms_is_member(attidx, external_cols))
            *has_external = true;
    }
 

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 20d23696a5357247b90cdfc20b15fa5b418ea001..5960e4a6c19a2dae296688926cf56a4efc0e4521 100644 (file)
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -1646,7 +1646,8 @@ find_hash_columns(AggState *aggstate)
        }
 
        /* and add the remaining columns */
-       while ((i = bms_first_member(colnos)) >= 0)
+       i = -1;
+       while ((i = bms_next_member(colnos, i)) >= 0)
        {
            perhash->hashGrpColIdxInput[perhash->numhashGrpCols] = i;
            perhash->numhashGrpCols++;

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index 98660524adefe4362832e941c2169b464c817a1f..10dbbdddf58a32d2b987c2efd1b3e6d5e0c0932a 100644 (file)
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -982,48 +982,6 @@ 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!
- *
- * This is intended as support for iterating through the members of a set.
- * The typical pattern is
- *
- *         while ((x = bms_first_member(inputset)) >= 0)
- *             process member x;
- *
- * 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)
-{
-   int         nwords;
-   int         wordnum;
-
-   if (a == NULL)
-       return -1;
-   nwords = a->nwords;
-   for (wordnum = 0; wordnum < nwords; wordnum++)
-   {
-       bitmapword  w = a->words[wordnum];
-
-       if (w != 0)
-       {
-           int         result;
-
-           w = RIGHTMOST_ONE(w);
-           a->words[wordnum] &= ~w;
-
-           result = wordnum * BITS_PER_BITMAPWORD;
-           result += bmw_rightmost_one_pos(w);
-           return result;
-       }
-   }
-   return -1;
-}
-
 /*
  * bms_next_member - find next member of a set
  *

diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 04eda4798efeab219711b81c5c127f5375321cf4..22ffe4ca425af29d4afa9f404acdf9e5b7407a3c 100644 (file)
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -1481,7 +1481,8 @@ convert_EXISTS_sublink_to_join(PlannerInfo *root, SubLink *sublink,
     */
    clause_varnos = pull_varnos(root, whereClause);
    upper_varnos = NULL;
-   while ((varno = bms_first_member(clause_varnos)) >= 0)
+   varno = -1;
+   while ((varno = bms_next_member(clause_varnos, varno)) >= 0)
    {
        if (varno <= rtoffset)
            upper_varnos = bms_add_member(upper_varnos, varno);

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 7ff41acb846315d9d405f862b2d91ac528df1735..78221d2e0f7a863d60d2d664faa3d69d48190688 100644 (file)
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -2759,7 +2759,7 @@ make_row_comparison_op(ParseState *pstate, List *opname,
     * them ... this coding arbitrarily picks the lowest btree strategy
     * number.
     */
-   i = bms_first_member(strats);
+   i = bms_next_member(strats, -1);
    if (i < 0)
    {
        /* No common interpretation, so fail */

diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 9f139c64dba7ad4865688326e58f64f5b800d93d..55bfa078711529a1476f1c6ba881b4d9cdab8efc 100644 (file)
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -227,7 +227,8 @@ logicalrep_report_missing_attrs(LogicalRepRelation *remoterel,
 
        initStringInfo(&missingattsbuf);
 
-       while ((i = bms_first_member(missingatts)) >= 0)
+       i = -1;
+       while ((i = bms_next_member(missingatts, i)) >= 0)
        {
            missingattcnt++;
            if (missingattcnt == 1)

diff --git a/src/include/nodes/bitmapset.h b/src/include/nodes/bitmapset.h
index 3d2225e1ae05240945ed351beb6f499b1fe06218..c344ac04bec1e50fe39996bc240b35b185be7dd0 100644 (file)
--- a/src/include/nodes/bitmapset.h
+++ b/src/include/nodes/bitmapset.h
@@ -115,7 +115,6 @@ extern Bitmapset *bms_del_members(Bitmapset *a, const Bitmapset *b);
 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);
 extern int bms_prev_member(const Bitmapset *a, int prevbit);