On Sat, May 30, 2020 at 12:13 AM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
>
>
> I forgot in my first round of code review to mention, "thanks for the patch". I generally like what you are doing
here,and am trying to review it so it gets committed.
And I forgot to say thanks for taking a look!
> The reason I gave this feedback is that I saved the *kwlist_d.h files generated before applying the patch, and
comparedthem with the same files generated after applying the patch, and noticed a very slight degradation. Most of
thefiles changed without any expansion, but the largest of them, src/common/kwlist_d.h, changed from
>
> static const int16 h[901]
>
> to
>
> static const int16 h[902]
Interesting, I hadn't noticed. With 450 keywords, we need at least 901
elements in the table. Since 901 is divisible by the new hash
multiplier 17, this gets triggered:
# However, it would be very bad if $nverts were exactly equal to either
# $hash_mult1 or $hash_mult2: effectively, that hash function would be
# sensitive to only the last byte of each key. Cases where $nverts is a
# multiple of either multiplier likewise lose information. (But $nverts
# can't actually divide them, if they've been intelligently chosen as
# primes.) We can avoid such problems by adjusting the table size.
while ($nverts % $hash_mult1 == 0
|| $nverts % $hash_mult2 == 0)
{
$nverts++;
}
This is harmless, and will go away next time we add a keyword.
--
John Naylor https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services