On Thu, Nov 3, 2016 at 2:52 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Thu, Nov 3, 2016 at 6:15 PM, Kuntal Ghosh <kuntalghosh.2007@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