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: /messages/by-id/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
Attachments:
bufpage-macro.patchtext/x-diff; charset=us-asciiDownload+1-1
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
On Mon, Oct 26, 2020 at 09:29:46PM -0400, Tom Lane wrote:
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.
Thanks. I really thought that this remark was for the bit-flag
argument for readability reasons, not the two others as we don't do
that in other places (varbit.h, bufpage.h, timestamp.h, brin_page.h,
etc.). Perhaps I just missed the point.
--
Michael