On 2012-12-18 13:14:10 +0100, Dimitri Fontaine wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> > In 2) unfortunately one has to make decision in which way to simplify
> > negated XLByte(LT|LE) expressions. I tried to make that choice very
> > careful and when over every change several times after that, so I hope
> > there aren't any bad changes, but more eyeballs are needed.
>
> + if (lsn > PageGetLSN(page))
> + if (lsn >= PageGetLSN(page))
> + if (lsn <= PageGetLSN(page))
> + if (max_lsn < this_lsn)
>
> My only comment here would be that I would like it much better that the
> code always use the same operators, and as we read from left to right,
> that we pick < and <=.
I did that in most places (check the xlog.c changes), but in this case
it didn't seem to be appropriate because because that would have meant
partially reversing the order of operands which would have looked
confusing as well, given this check is a common patter across nearly
every xlog replay function.
Imo its a good idea trying to choose < or <= as operator but its also
important to keep the order consistent inside a single function/similar
functions.
Greetings,
Andres Freund
--Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
