Tom Lane <t...@sss.pgh.pa.us> wrote: > Kevin Grittner <kgri...@ymail.com> writes: >> Kouhei Kaigai <kai...@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? There is a 1 shifted for nbuckets that I didn't touch because it could not. The limits on nbuckets are not applied at the point of the bug. > (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? In such cases there is not a bug -- at least not with my_log returning less than 63. In those cases the change will make no difference. > 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. You seem to have missed reading the line in between: | lbuckets = 1L << my_log2(hash_table_bytes / bucket_size); | lbuckets = Min(lbuckets, max_pointers); | nbuckets = (int) lbuckets; There is a Min() and a cast to sort that out. > So I think the "1" coding was just fine in context. I disagree. > If there is an overflow in ExecChooseHashTableSize, it's > somewhere else. I do believe that there are other places we need to place limits, to avoid attempts to allocate more than 1GB, but I think patches to address that should be separate from fixing this particular bug. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
