On 2012-11-10 16:24:22 -0500, Tom Lane wrote:
> Simon Riggs <simon@2ndQuadrant.com> writes:
> >> One thing that seems a bit annoying is the use of zero-based backup
> >> block indexes in RestoreBackupBlock, when most (not all) of the callers
> >> are referencing macros with one-based names (XLR_BKP_BLOCK_1 etc).
> >> That's a bug waiting to happen. We could address it by changing
> >> RestoreBackupBlock to accept a one-based argument, but what I'm thinking
> >> of doing instead is getting rid of the one-based convention entirely;
> >> that is, remove XLR_BKP_BLOCK_1 through XLR_BKP_BLOCK_4 and instead have
> >> all code use the XLR_SET_BKP_BLOCK() macro, which is zero-based. One
> >> advantage of doing that is that it'll force inspection of all code
> >> related to this.
>
> > I wouldn't do that in a back branch, but I can see why its a good idea.
>
> If any of this stuff were getting used by external modules, changing it
> would be problematic ... but fortunately, it isn't, because we lack
> support for plug-in rmgrs. So I'm not worried about back-patching the
> change, and would prefer to keep the 9.x branches in sync.
XLR_BKP_BLOCK_* might be used by things like pg_lesslog and its surely
used by xlogdump. Not sure if either are worth that much attention, but
it seems worth noticing that such a change will break stuff.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
