Re: Bug? ExecChooseHashTableSize() got assertion failed with crazy number of rows - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Bug? ExecChooseHashTableSize() got assertion failed with crazy number of rows
Date
Msg-id 32407.1439995540@sss.pgh.pa.us
Whole thread Raw
In response to Re: Bug? ExecChooseHashTableSize() got assertion failed with crazy number of rows  (Kevin Grittner <kgrittn@ymail.com>)
Responses Re: Bug? ExecChooseHashTableSize() got assertion failed with crazy number of rows  (Kevin Grittner <kgrittn@ymail.com>)
Re: Bug? ExecChooseHashTableSize() got assertion failed with crazy number of rows  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Kevin Grittner <kgrittn@ymail.com> writes:
> Kouhei Kaigai <kaigai@ak.jp.nec.com> wrote:
>> we may need a couple of overhaul around HashJoin to support large
>> size of data, not only nbuckets around 0x80000000.

> Perhaps, but this is a clear bug, introduced to the 9.5 code, with
> an obvious fix; so I've pushed the change from 1 to 1L on that left
> shift.

I don't think it's anywhere near as clear as you think.  The preceding
lines should have limited nbuckets to be no more than INT_MAX/2, so how
could an overflow occur there?  (The result of 1<<mylog() should be
at most 0x40000000 AFAICS.)  If overflow is possible, how will s/1/1L/
make it better on machines where int and long are the same size?

And on machines where long is wider than int, you've still got a bug
if the result of the left shift somehow overflowed an int, because
it's going to be assigned to nbuckets which is an int.

So I think the "1" coding was just fine in context.  If there is an
overflow in ExecChooseHashTableSize, it's somewhere else.
        regards, tom lane



pgsql-hackers by date:

Previous
From: ''Victor Wagner *EXTERN*' *EXTERN*' *EXTERN*
Date:
Subject: Re: Proposal: Implement failover on libpq connect level.
Next
From: Simon Riggs
Date:
Subject: Re: Make HeapTupleSatisfiesMVCC more concurrent