On 2012-11-10 16:24:22 -0500, Tom Lane wrote: > Simon Riggs <si...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
