Thread: Applying XLR_INFO_MASK correctly when looking at WAL record information
Hi all, I have just been trapped by the fact that in some code paths we look at the RMGR information of a WAL record (record->xl_info) but the filter dedicated to it, ~XLR_INFO_MASK, is not applied. This is harmful now, but this could lead to problems if in the future new record-level flags, similar to XLR_SPECIAL_REL_UPDATE, are added. Disclaimer: I am looking at a patch where a record-level flag makes sense, still this looks like a separate problem. What about the patch attached to make things more consistent? Thanks, -- Michael
Attachment
Re: Applying XLR_INFO_MASK correctly when looking at WAL record information
From
Ashutosh Sharma
Date:
Hi, > What about the patch attached to make things more consistent? I have reviewed this patch. It does serve the purpose and looks sane to me. I am marking it as ready for committer. With Regards, Ashutosh Sharma EnterpriseDB: http://www.enterprisedb.com
Michael Paquier <michael.paquier@gmail.com> writes:
> I have just been trapped by the fact that in some code paths we look
> at the RMGR information of a WAL record (record->xl_info) but the
> filter dedicated to it, ~XLR_INFO_MASK, is not applied. This is
> harmful now, but this could lead to problems if in the future new
> record-level flags, similar to XLR_SPECIAL_REL_UPDATE, are added.
> Disclaimer: I am looking at a patch where a record-level flag makes
> sense, still this looks like a separate problem.
> What about the patch attached to make things more consistent?
Grepping found a couple of additional places that needed the same
fix. Pushed with those additions.
regards, tom lane
Re: Applying XLR_INFO_MASK correctly when looking at WAL record information
From
Michael Paquier
Date:
On Sat, Nov 5, 2016 at 2:29 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Michael Paquier <michael.paquier@gmail.com> writes: >> I have just been trapped by the fact that in some code paths we look >> at the RMGR information of a WAL record (record->xl_info) but the >> filter dedicated to it, ~XLR_INFO_MASK, is not applied. This is >> harmful now, but this could lead to problems if in the future new >> record-level flags, similar to XLR_SPECIAL_REL_UPDATE, are added. >> Disclaimer: I am looking at a patch where a record-level flag makes >> sense, still this looks like a separate problem. > >> What about the patch attached to make things more consistent? > > Grepping found a couple of additional places that needed the same > fix. Ah, where wasShutdown is assigned. I thought I grepped it as well, thanks for double-checking. > Pushed with those additions. Thanks. -- Michael