On Sat, Jan 18, 2020 at 11:46:24AM +0800, John Naylor wrote:
> On Wed, Jan 15, 2020 at 6:09 AM David Fetter <david@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.
Fixed by removing the variable entirely.
> 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.
Removed.
Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate