Fix a couple of misuages of bms_num_members()

Started by David Rowleyover 5 years ago3 messages
#1David Rowley
dgrowleyml@gmail.com
1 attachment(s)

I noticed today there are a few places where we use bms_num_memebers()
where we really should be using bms_membership(). These are not bugs,
they're mostly just bad examples to leave laying around, at best, and
a small performance penalty, at worst.

Unless there are any objections, I plan to push this to master only in
about 10 hours time.

David

Attachments:

fixup_bms_num_members_misusages.patchapplication/octet-stream; name=fixup_bms_num_members_misusages.patchDownload
diff --git a/src/backend/optimizer/path/clausesel.c b/src/backend/optimizer/path/clausesel.c
index a3ebe10592..37a735b06b 100644
--- a/src/backend/optimizer/path/clausesel.c
+++ b/src/backend/optimizer/path/clausesel.c
@@ -164,8 +164,7 @@ clauselist_selectivity_simple(PlannerInfo *root,
 	 * directly to clause_selectivity(). None of what we might do below is
 	 * relevant.
 	 */
-	if ((list_length(clauses) == 1) &&
-		bms_num_members(estimatedclauses) == 0)
+	if (list_length(clauses) == 1 && bms_is_empty(estimatedclauses))
 		return clause_selectivity(root, (Node *) linitial(clauses),
 								  varRelid, jointype, sjinfo);
 
diff --git a/src/backend/statistics/dependencies.c b/src/backend/statistics/dependencies.c
index 3e37e2758c..4e30abb674 100644
--- a/src/backend/statistics/dependencies.c
+++ b/src/backend/statistics/dependencies.c
@@ -1246,7 +1246,7 @@ dependencies_clauselist_selectivity(PlannerInfo *root,
 	 * of clauses. We must return 1.0 so the calling function's selectivity is
 	 * unaffected.
 	 */
-	if (bms_num_members(clauses_attnums) < 2)
+	if (bms_membership(clauses_attnums) != BMS_MULTIPLE)
 	{
 		bms_free(clauses_attnums);
 		pfree(list_attnums);
@@ -1273,18 +1273,18 @@ dependencies_clauselist_selectivity(PlannerInfo *root,
 	{
 		StatisticExtInfo *stat = (StatisticExtInfo *) lfirst(l);
 		Bitmapset  *matched;
-		int			num_matched;
+		BMS_Membership membership;
 
 		/* skip statistics that are not of the correct type */
 		if (stat->kind != STATS_EXT_DEPENDENCIES)
 			continue;
 
 		matched = bms_intersect(clauses_attnums, stat->keys);
-		num_matched = bms_num_members(matched);
+		membership = bms_membership(matched);
 		bms_free(matched);
 
 		/* skip objects matching fewer than two attributes from clauses */
-		if (num_matched < 2)
+		if (membership != BMS_MULTIPLE)
 			continue;
 
 		func_dependencies[nfunc_dependencies]
#2Tomas Vondra
tomas.vondra@2ndquadrant.com
In reply to: David Rowley (#1)
Re: Fix a couple of misuages of bms_num_members()

On Wed, Aug 26, 2020 at 12:51:37AM +1200, David Rowley wrote:

I noticed today there are a few places where we use bms_num_memebers()
where we really should be using bms_membership(). These are not bugs,
they're mostly just bad examples to leave laying around, at best, and
a small performance penalty, at worst.

Unless there are any objections, I plan to push this to master only in
about 10 hours time.

Seems OK to me. Thanks.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#3David Rowley
dgrowleyml@gmail.com
In reply to: Tomas Vondra (#2)
Re: Fix a couple of misuages of bms_num_members()

On Wed, 26 Aug 2020 at 01:18, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:

On Wed, Aug 26, 2020 at 12:51:37AM +1200, David Rowley wrote:

I noticed today there are a few places where we use bms_num_memebers()
where we really should be using bms_membership(). These are not bugs,
they're mostly just bad examples to leave laying around, at best, and
a small performance penalty, at worst.

Unless there are any objections, I plan to push this to master only in
about 10 hours time.

Seems OK to me. Thanks.

Thanks for having a look. Pushed.

David