On Wed, Feb 1, 2017 at 8:01 AM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Wed, Feb 1, 2017 at 1:06 AM, Robert Haas <robertmh...@gmail.com> wrote:
>> + /* >> + * Leave if no masking functions defined, this is possible in the case >> + * resource managers generating just full page writes, comparing an >> + * image to itself has no meaning in those cases. >> + */ >> + if (RmgrTable[rmid].rm_mask == NULL) >> + return; >> >> ...and also... >> >> + /* >> + * This feature is enabled only for the resource managers where >> + * a masking function is defined. >> + */ >> + for (i = 0; i <= RM_MAX_ID; i++) >> + { >> + if (RmgrTable[i].rm_mask != NULL) >> >> Why do we assume that the feature is only enabled for RMs that have a >> mask function? Why not instead assume that if there's no masking >> function, no masking is required? > > Not all RMs work on full pages. Tracking in smgr.h the list of RMs > that are no-ops when masking things is easier than having empty > routines declared all over the code base. Don't you think so> > Robert's suggestion surely makes the approach more general. But, the existing approach makes it easier to decide the RM's for which we support the consistency check facility. Surely, we can use a list to track the RMs which should (/not) be masked. But, I think we always have to mask the lsn of the pages at least. Isn't it? >> + void (*rm_mask) (char *page, BlockNumber blkno); >> >> Could the page be passed as type "Page" rather than a "char *" to make >> things more convenient for the masking functions? If not, could those >> functions at least do something like "Page page = (Page) pagebytes;" >> rather than "Page page_norm = (Page) page;"? > > xlog_internal.h is used as well by frontends, this makes the header > dependencies cleaner. (I have looked at using Page when hacking this > stuff, but the header dependencies are not worth it, I don't recall > all the details though this was a couple of months back). + 1 -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers