On Thu, Jun 14, 2018 at 08:14:54PM +0000, Bossart, Nathan wrote:
> I'll go ahead and mark this as Ready for Committer.

Another thing not mentioned on this thread is that bms_membership is
faster than bms_num_members by design with many members, so this change
makes sense to shave a couple of cycles.

 /*
  * bms_num_members - count members of set
+ *
+ * In situations where the exact count isn't required, bms_membership can be
+ * used to test if the set has 0, 1 or multiple members.
   */
bms_membership is a couple of lines down, I am not sure I see much point
in duplicating what's already present.

-   if (bms_num_members(clauses_attnums) < 2)
+   if (bms_membership(clauses_attnums) != BMS_MULTIPLE)
For this one, the comment above directly mentions that at least two
attnums need to be present, so it seems to me that the current coding is
easier to understand and intentional...  So I would be incline to not
change it.

I think that this should not go into the tree until REL_11_STABLE gets
created though, so this will have to wait a bit.
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to