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; Having only looked at this properly now, I don't think that that's actually the case, but I thought that it should be noted. I'm sorry if it's unfair to Greg to correct him, even though that's something he didn't formally put on the record, and may have only looked at briefly. I can see why it might have looked like a concern, since we're assigning the result of an arithmetic expression involving long variables to an int, newmemtupsize. The fact of the matter is that we're already asserting that: + Assert(newmemtupsize <= state->memtupsize * 2); Previously, we'd just have doubled memtupsize anyway. So any eventuality in which newmemtupsize overflows ought to be logically impossible. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers