From: | "Todd A(dot) Cook" <tcook(at)blackducksoftware(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: simplehash: tb->sizemask = 0 |
Date: | 2017-12-05 14:49:46 |
Message-ID: | 701a0e01-480e-a809-8c6b-e62980daa31b@blackducksoftware.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 11/29/17 13:49, Andres Freund wrote:
> On 2017-11-27 22:53:41 -0500, Tom Lane wrote:
>> Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
>>> I'm a bit puzzled by this code in SH_COMPUTE_PARAMETERS:
>>
>>> if (tb->size == SH_MAX_SIZE)
>>> tb->sizemask = 0;
>>> else
>>> tb->sizemask = tb->size - 1;
>>
>>> Doesn't that mean that with SH_MAX_SIZE we end up with sizemask being 0
>>> (i.e. no bits set)?
>>
>> Yeah, which is very obviously broken: for one thing, the Asserts
>> in SH_NEXT/SH_PREV would surely go off.
>
> That's obviously wrong. Not sure how that happened. I might have had it
> as a shift at first?
>
>
>> (Why are those assertions, anyway, and not test-and-elog?
>> I do not think an assertion failure is a suitable way to
>> report "hash table full".)
>
> There's a test and elog during insert. Adding actual branches into
> SH_NEXT/SH_PREV seems like a bad idea.
>
> Will test a fix.
I'll be happy to help test this fix when it's ready.
-- todd
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2017-12-05 15:29:59 | Re: [HACKERS] Proposal: Local indexes for partitioned table |
Previous Message | Robert Haas | 2017-12-05 14:49:10 | Re: Add PGDLLIMPORT lines to some variables |