Re: a slightly stale comment

Simon Riggs <simon@2ndQuadrant.com> wrote:
> Dan Ports <drkp@csail.mit.edu> wrote:
>> While mucking around in src/backend/utils/time/tqual.c today, I
>> noticed the following comment attached to HeapTupleSatisfiesNow:
>>  [a comment explaining that if you think the code needs to be
>>   changed, you are wrong, because 2 phase locking will prevent
>>   any problems]
>> Much as I hate to disturb a comment just before its 19th
>> birthday, the bit about two-phase locking and serializability
>> hasn't been correct since around 1999 (when MVCC was added). :-)
> 
> There is much wisdom there and much wisdom in leaving ancient
> warnings as we find them.
What?  That comment makes multiple references to 2 phase locking,
which was supplanted by MVCC, and the tests in that routine have
been rather heavily modified since that warning was added -- so
apparently the tests have become wrong several times since then.
If the point is that great care should be taken in modifying this
section of code, and you think it is significantly more fragile than
other code, then a comment explaining why that is currently true
would be a good thing; but I don't see any value in littering the
code with comments about why it might have been true before MVCC was
implemented.
> Are these the words you object to?
> 
>>  *      we don't need to check commit time against the start time
>>  *      of this transaction because 2ph locking protects us from
>>  *      doing the wrong thing.
Well, certainly any reference to 2 phase locking would be wrong. 
What part of it do you find to be accurate or helpful?
-Kevin