On Thu, Nov 15, 2012 at 12:14 PM, Peter Geoghegan <pe...@2ndquadrant.com> wrote: > On 15 November 2012 16:09, Robert Haas <robertmh...@gmail.com> wrote: > [Lots of complicated commentary on tuplesort variables] >> Whew. In the attached version, I >> updated the comment to reflect this, and also reversed the order of >> the if/else block to put the shorter and more common case first, which >> I think is better style. > > Fine by me. > > In conversation with Greg Stark in Prague, he seemed to think that > there was an integer overflow hazard in v3, which is, in terms of the > machine code it will produce, identical to your revision. He pointed > this out to me when we were moving between sessions, and I only > briefly looked over his shoulder, so while I don't want to > misrepresent what he said, I *think* he said that this could overflow: > > + newmemtupsize = memtupsize * allowedMem / memNowUsed;
Ah, yeah. I wondered in passing about that but forgot to follow up on it. The problem specifically is that the intermediate result memtupsize * newmemtuples might overflow. I believe that the old memtupsize can never be more than 2^26 bytes, because the allocation limit is 1GB and each SortTuple is 16 bytes. So if this is done as a 32-bit calculation, we'll potentially overflow if allowedMem is more than 64 bytes i.e. almost for sure. If it is done as a 64-byte calculation we'll potentially overflow if allowedMem is more than 2^38 bytes = 256GB. The actual declared type is "long" which I assume is probably 64 bits at least for most people these days, but maybe not for everyone, and even 256GB is not necessarily safe. The highest value for work_mem I can set here is 2047GB. So I think there is an issue here, unless there's something wrong with my analysis. -- Robert Haas EnterpriseDB: 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
