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


Reply via email to