On 15 November 2012 16:09, Robert Haas <robertmhaas@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
