Thread: pgsql: Extend PageIsVerified() to handle more custom options
Extend PageIsVerified() to handle more custom options This is useful for checks of relation pages without having to load the pages into the shared buffers, and two cases can make use of that: page verification in base backups and the online, lock-safe, flavor. Compatibility is kept with past versions using a macro that calls the new extended routine with the set of options compatible with the original version. Extracted from a larger patch by the same author. Author: Anastasia Lubennikova Reviewed-by: Michael Paquier, Julien Rouhaud Discussion: https://postgr.es/m/608f3476-0598-2514-2c03-e05c7d2b0cbd@postgrespro.ru Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/d401c5769ef6aeef0a28c147f3fb5afedcd59984 Modified Files -------------- src/backend/catalog/storage.c | 3 ++- src/backend/storage/buffer/bufmgr.c | 6 ++++-- src/backend/storage/page/bufpage.c | 22 +++++++++++++++------- src/include/storage/bufpage.h | 22 ++++++++++++++++------ 4 files changed, 37 insertions(+), 16 deletions(-)
On 2020-Oct-26, Michael Paquier wrote: > Extend PageIsVerified() to handle more custom options > > This is useful for checks of relation pages without having to load the > pages into the shared buffers, and two cases can make use of that: page > verification in base backups and the online, lock-safe, flavor. > > Compatibility is kept with past versions using a macro that calls the > new extended routine with the set of options compatible with the > original version. Please remember that in the macro definition, the arguments should be enclosed in parens. No bug here at present, but it seems better to be cautious.
On Mon, Oct 26, 2020 at 11:39:33AM -0300, Alvaro Herrera wrote: > Please remember that in the macro definition, the arguments should be > enclosed in parens. No bug here at present, but it seems better to be > cautious. Indeed, I can see similar changes in the history of the tree. Do you think that doing something like the attached is sufficient? -- Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes:
> On Mon, Oct 26, 2020 at 11:39:33AM -0300, Alvaro Herrera wrote:
>> Please remember that in the macro definition, the arguments should be
>> enclosed in parens. No bug here at present, but it seems better to be
>> cautious.
> Indeed, I can see similar changes in the history of the tree. Do you
> think that doing something like the attached is sufficient?
Hm, I suspect what Alvaro meant was
#define PageIsVerified(page, blkno) \
- PageIsVerifiedExtended(page, blkno, \
+ PageIsVerifiedExtended((page), (blkno), \
But IMV that's not necessary: if the argument parsed as a single
function/macro argument before, it still will do so. The places
where you have to be careful to add parentheses are where the
macro argument is used as part of a larger expression that is
not just a function/macro argument.
regards, tom lane