Skip to content

Commit 6185f78

Browse files
tglsfdcpull[bot]
tglsfdc
authored and
pull[bot]
committed
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
1 parent 70b4228 commit 6185f78

File tree

9 files changed

+23
-70
lines changed

9 files changed

+23
-70
lines changed

‎contrib/file_fdw/file_fdw.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,7 @@ check_selective_binary_conversion(RelOptInfo *baserel,
858858
ListCell *lc;
859859
Relation rel;
860860
TupleDesc tupleDesc;
861-
AttrNumber attnum;
861+
int attidx;
862862
Bitmapset *attrs_used = NULL;
863863
bool has_wholerow = false;
864864
int numattrs;
@@ -901,10 +901,11 @@ check_selective_binary_conversion(RelOptInfo *baserel,
901901
rel = table_open(foreigntableid, AccessShareLock);
902902
tupleDesc = RelationGetDescr(rel);
903903

904-
while ((attnum = bms_first_member(attrs_used)) >= 0)
904+
attidx = -1;
905+
while ((attidx = bms_next_member(attrs_used, attidx)) >= 0)
905906
{
906-
/* Adjust for system attributes. */
907-
attnum += FirstLowInvalidHeapAttributeNumber;
907+
/* attidx is zero-based, attnum is the normal attribute number */
908+
AttrNumber attnum = attidx + FirstLowInvalidHeapAttributeNumber;
908909

909910
if (attnum == 0)
910911
{

‎contrib/sepgsql/dml.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,8 @@ check_relation_privileges(Oid relOid,
231231
updated = fixup_whole_row_references(relOid, updated);
232232
columns = bms_union(selected, bms_union(inserted, updated));
233233

234-
while ((index = bms_first_member(columns)) >= 0)
234+
index = -1;
235+
while ((index = bms_next_member(columns, index)) >= 0)
235236
{
236237
AttrNumber attnum;
237238
uint32 column_perms = 0;

‎src/backend/access/heap/heapam.c

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3859,9 +3859,6 @@ heap_attr_equals(TupleDesc tupdesc, int attrnum, Datum value1, Datum value2,
38593859
* has_external indicates if any of the unmodified attributes (from those
38603860
* listed as interesting) of the old tuple is a member of external_cols and is
38613861
* stored externally.
3862-
*
3863-
* The input interesting_cols bitmapset is destructively modified; that is OK
3864-
* since this is invoked at most once in heap_update.
38653862
*/
38663863
static Bitmapset *
38673864
HeapDetermineColumnsInfo(Relation relation,
@@ -3870,29 +3867,28 @@ HeapDetermineColumnsInfo(Relation relation,
38703867
HeapTuple oldtup, HeapTuple newtup,
38713868
bool *has_external)
38723869
{
3873-
int attrnum;
3870+
int attidx;
38743871
Bitmapset *modified = NULL;
38753872
TupleDesc tupdesc = RelationGetDescr(relation);
38763873

3877-
while ((attrnum = bms_first_member(interesting_cols)) >= 0)
3874+
attidx = -1;
3875+
while ((attidx = bms_next_member(interesting_cols, attidx)) >= 0)
38783876
{
3877+
/* attidx is zero-based, attrnum is the normal attribute number */
3878+
AttrNumber attrnum = attidx + FirstLowInvalidHeapAttributeNumber;
38793879
Datum value1,
38803880
value2;
38813881
bool isnull1,
38823882
isnull2;
38833883

3884-
attrnum += FirstLowInvalidHeapAttributeNumber;
3885-
38863884
/*
38873885
* If it's a whole-tuple reference, say "not equal". It's not really
38883886
* worth supporting this case, since it could only succeed after a
38893887
* no-op update, which is hardly a case worth optimizing for.
38903888
*/
38913889
if (attrnum == 0)
38923890
{
3893-
modified = bms_add_member(modified,
3894-
attrnum -
3895-
FirstLowInvalidHeapAttributeNumber);
3891+
modified = bms_add_member(modified, attidx);
38963892
continue;
38973893
}
38983894

@@ -3905,9 +3901,7 @@ HeapDetermineColumnsInfo(Relation relation,
39053901
{
39063902
if (attrnum != TableOidAttributeNumber)
39073903
{
3908-
modified = bms_add_member(modified,
3909-
attrnum -
3910-
FirstLowInvalidHeapAttributeNumber);
3904+
modified = bms_add_member(modified, attidx);
39113905
continue;
39123906
}
39133907
}
@@ -3924,9 +3918,7 @@ HeapDetermineColumnsInfo(Relation relation,
39243918
if (!heap_attr_equals(tupdesc, attrnum, value1,
39253919
value2, isnull1, isnull2))
39263920
{
3927-
modified = bms_add_member(modified,
3928-
attrnum -
3929-
FirstLowInvalidHeapAttributeNumber);
3921+
modified = bms_add_member(modified, attidx);
39303922
continue;
39313923
}
39323924

@@ -3943,8 +3935,7 @@ HeapDetermineColumnsInfo(Relation relation,
39433935
* member of external_cols.
39443936
*/
39453937
if (VARATT_IS_EXTERNAL((struct varlena *) DatumGetPointer(value1)) &&
3946-
bms_is_member(attrnum - FirstLowInvalidHeapAttributeNumber,
3947-
external_cols))
3938+
bms_is_member(attidx, external_cols))
39483939
*has_external = true;
39493940
}
39503941

‎src/backend/executor/nodeAgg.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1646,7 +1646,8 @@ find_hash_columns(AggState *aggstate)
16461646
}
16471647

16481648
/* and add the remaining columns */
1649-
while ((i = bms_first_member(colnos)) >= 0)
1649+
i = -1;
1650+
while ((i = bms_next_member(colnos, i)) >= 0)
16501651
{
16511652
perhash->hashGrpColIdxInput[perhash->numhashGrpCols] = i;
16521653
perhash->numhashGrpCols++;

‎src/backend/nodes/bitmapset.c

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -982,48 +982,6 @@ bms_join(Bitmapset *a, Bitmapset *b)
982982
return result;
983983
}
984984

985-
/*
986-
* bms_first_member - find and remove first member of a set
987-
*
988-
* Returns -1 if set is empty. NB: set is destructively modified!
989-
*
990-
* This is intended as support for iterating through the members of a set.
991-
* The typical pattern is
992-
*
993-
* while ((x = bms_first_member(inputset)) >= 0)
994-
* process member x;
995-
*
996-
* CAUTION: this destroys the content of "inputset". If the set must
997-
* not be modified, use bms_next_member instead.
998-
*/
999-
int
1000-
bms_first_member(Bitmapset *a)
1001-
{
1002-
int nwords;
1003-
int wordnum;
1004-
1005-
if (a == NULL)
1006-
return -1;
1007-
nwords = a->nwords;
1008-
for (wordnum = 0; wordnum < nwords; wordnum++)
1009-
{
1010-
bitmapword w = a->words[wordnum];
1011-
1012-
if (w != 0)
1013-
{
1014-
int result;
1015-
1016-
w = RIGHTMOST_ONE(w);
1017-
a->words[wordnum] &= ~w;
1018-
1019-
result = wordnum * BITS_PER_BITMAPWORD;
1020-
result += bmw_rightmost_one_pos(w);
1021-
return result;
1022-
}
1023-
}
1024-
return -1;
1025-
}
1026-
1027985
/*
1028986
* bms_next_member - find next member of a set
1029987
*

‎src/backend/optimizer/plan/subselect.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1481,7 +1481,8 @@ convert_EXISTS_sublink_to_join(PlannerInfo *root, SubLink *sublink,
14811481
*/
14821482
clause_varnos = pull_varnos(root, whereClause);
14831483
upper_varnos = NULL;
1484-
while ((varno = bms_first_member(clause_varnos)) >= 0)
1484+
varno = -1;
1485+
while ((varno = bms_next_member(clause_varnos, varno)) >= 0)
14851486
{
14861487
if (varno <= rtoffset)
14871488
upper_varnos = bms_add_member(upper_varnos, varno);

‎src/backend/parser/parse_expr.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2759,7 +2759,7 @@ make_row_comparison_op(ParseState *pstate, List *opname,
27592759
* them ... this coding arbitrarily picks the lowest btree strategy
27602760
* number.
27612761
*/
2762-
i = bms_first_member(strats);
2762+
i = bms_next_member(strats, -1);
27632763
if (i < 0)
27642764
{
27652765
/* No common interpretation, so fail */

‎src/backend/replication/logical/relation.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,8 @@ logicalrep_report_missing_attrs(LogicalRepRelation *remoterel,
227227

228228
initStringInfo(&missingattsbuf);
229229

230-
while ((i = bms_first_member(missingatts)) >= 0)
230+
i = -1;
231+
while ((i = bms_next_member(missingatts, i)) >= 0)
231232
{
232233
missingattcnt++;
233234
if (missingattcnt == 1)

‎src/include/nodes/bitmapset.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ extern Bitmapset *bms_del_members(Bitmapset *a, const Bitmapset *b);
115115
extern Bitmapset *bms_join(Bitmapset *a, Bitmapset *b);
116116

117117
/* support for iterating through the integer elements of a set: */
118-
extern int bms_first_member(Bitmapset *a);
119118
extern int bms_next_member(const Bitmapset *a, int prevbit);
120119
extern int bms_prev_member(const Bitmapset *a, int prevbit);
121120

0 commit comments

Comments
 (0)