On Tue, May 29, 2018 at 9:10 PM, Daniel Gustafsson <dan...@yesql.se> wrote: > There are a couple of places in postgres_fdw where we check if the Bitmapset > has multiple members using bms_num_members(), without storing the returned > count. The attached patch instead use bms_membership() which is optimized for > just that usecase, and (IMO) makes for clearer code. >
+1 for that change. Some of those usages of bms_num_members() were added by me. Sorry for that. It was mostly because I wasn't aware of bms_membership() when I wrote that code. May be we should add a comment in the prologue of bms_num_members() like "Note: if the callers is interested only knowing whether the bitmapset has 0, 1 or more members, it should call bms_membership().". I understand that bms_membership() resides just below bms_num_members(), but 1. bms_membership doesn't sound like it would tell me that 2. bms_membership's prologue refers to bms_num_members, which should have been the other way; we want the developers to use bms_membership instead of bms_num_members(), when they land on bms_num_members. It's less likely that somebody landing on bms_membership would want to use bms_num_members(). I am not sure if this can b e squeezed into HEAD right now. It looks safe to do so. But in case not, please add this to the next commitfest so that it's not forgotten. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
