Re: simplehash: tb->sizemask = 0 - Mailing list pgsql-hackers

From Andres Freund
Subject Re: simplehash: tb->sizemask = 0
Date
Msg-id 20171129184954.qtp34aepvamgpr7b@alap3.anarazel.de
Whole thread Raw
In response to Re: simplehash: tb->sizemask = 0  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: simplehash: tb->sizemask = 0  ("Todd A. Cook" <tcook@blackducksoftware.com>)
List pgsql-hackers
On 2017-11-27 22:53:41 -0500, Tom Lane wrote:
> Tomas Vondra <tomas.vondra@2ndquadrant.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.

Greetings,

Andres Freund


pgsql-hackers by date:

Previous
From: Emre Hasegeli
Date:
Subject: Re: [HACKERS] [PATCH] Improve geometric types
Next
From: Nikolay Shaplov
Date:
Subject: Re: [HACKERS] [PATCH] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind for custom AM