Re: Re: [COMMITTERS] pgsql: Adjust OLDSERXID_MAX_PAGE based on BLCKSZ.

Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Fri, Jul 8, 2011 at 10:57 AM, Heikki Linnakangas
>> <heikki.linnakangas@enterprisedb.com> wrote:
>>> So if MaxTransactionId+1 overflows to zero, OLDSERXID_MAX_PAGE
>>> becomes -1. Or a very high value, if the result of that is
>>> unsigned, as at least MSVC seems to interpret it given the other
>>> warning I got. If it's interpreted as a large unsigned value,
>>> then the SLRU_PAGES_PER_SEGMENT * 0x10000 - 1 value wins. That's
>>> what what we had prior to this patch, in beta2, so we're back to
>>> square one. If it's interpreted as signed -1, then bad things
>>> will happen as soon as the SLRU is used.
>
>> Should we, then, consider rewrapping beta3?
>
> At this point I think the actual choice we'd have is to abandon
> beta3 and try again next week with a beta4.  I'm trying to figure
> out whether this bug is serious enough to warrant that, but it's
> not clear to me.

I changed the definition to this:

#define OLDSERXID_MAX_PAGE (-1)

That caused my compiler to report the following warnings:

predicate.c: In function *OldSerXidAdd*:
predicate.c:828: warning: division by zero
predicate.c:848: warning: division by zero
predicate.c: In function *OldSerXidGetMinConflictCommitSeqNo*:
predicate.c:958: warning: division by zero
predicate.c: In function *CheckPointPredicate*:
predicate.c:1038: warning: division by zero

It's hard to imagine that any compiler would evaluate it to -1
instead of the value it's had all along (including beta2) and not
generate these warnings, too.

-Kevin