On Thu, Nov 3, 2016 at 2:52 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Thu, Nov 3, 2016 at 6:15 PM, Kuntal Ghosh <kuntalghosh.2...@gmail.com> > wrote: >> Actually, I just verified that bimg_info is not even valid if >> has_image is not set. >> In DecodeXLogRecord, we initialize bimg_info only when has_image flag >> is set. So, keeping them >> separate doesn't look a good approach to me. If we keep them separate, >> the output >> of the following assert is undefined: >> Assert(XLogRecHasBlockImage(record, block_id) || >> !XLogRecBlockImageApply(record, block_id)). >> >> Thoughts?? > > Yes, that's exactly the reason why we should keep both macros as > checking for separate things: apply implies that has_image is set and > that's normal, hence we could use sanity checks by just using those > macros and not propagating xlogreader.h. > No, apply doesn't mean has_image is set. If has_image is not set, apply/bimg_info is invalid(/undefined) and we should not use that. For example, in XLogDumpDisplayRecord we use bimg_info as following, if (XLogRecHasBlockImage(record, block_id)) { if (record->blocks[block_id].bimg_info & BKPIMAGE_IS_COMPRESSED) { } } So, whenever we are required to use bimg_info flag, we should make sure that has_image is set.
-- 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