On Wed, Jan 15, 2020 at 6:09 AM David Fetter <da...@fetter.org> wrote: > [v2 patch]
Hi David, I have a stylistic comment on this snippet: - for (i = _hash_log2(metap->hashm_bsize); i > 0; --i) - { - if ((1 << i) <= metap->hashm_bsize) - break; - } + i = pg_leftmost_one_pos32(metap->hashm_bsize); Assert(i > 0); metap->hashm_bmsize = 1 << i; metap->hashm_bmshift = i + BYTE_TO_BIT; Naming the variable "i" made sense when it was a loop counter, but it seems out of place now. Same with the Assert. Also, this + * using BSR where available */ is not directly tied to anything in this function, or even in the function it calls, and could get out of date easily. -- John Naylor https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services