Use return value of XLogRecGetBlockTag instead of explicit block ref checks and also use XLogRecHasBlockRef/Image macros instead of explicit checks

Started by Bharath Rupireddyalmost 4 years ago1 messages
#1Bharath Rupireddy
bharath.rupireddyforpostgres@gmail.com
1 attachment(s)

Hi,

I think we can use the return value of XLogRecGetBlockTag instead of
an explicit check XLogRecHasBlockRef as the XLogRecGetBlockTag would
anyways return false in such a case. Also, in some places the macros
XLogRecHasBlockRef and XLogRecHasBlockImage aren't being used instead
the checks are being performed directly.

Although the above change doesn't add any value or improve any
performance but makes use of the existing code better and removes some
unnecessary/duplicate code. Attaching a small patch.

Thoughts?

Regards,
Bharath Rupireddy.

Attachments:

v1-0001-Use-return-value-of-XLogRecGetBlockTag-instead-of.patchapplication/octet-stream; name=v1-0001-Use-return-value-of-XLogRecGetBlockTag-instead-of.patchDownload
From 3d9944939ac29bce828d78d58571016d48378280 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Thu, 10 Feb 2022 04:45:42 +0000
Subject: [PATCH v1] Use return value of XLogRecGetBlockTag instead of explicit
 block ref checks and also use XLogRecHasBlockRef/Image macros instead of
 explicit checks

---
 src/backend/access/transam/xlog.c       |  7 +++++--
 src/backend/access/transam/xlogreader.c |  8 ++++----
 src/bin/pg_waldump/pg_waldump.c         | 16 ++++++++++++----
 3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 958220c495..08e3a3d226 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -10707,11 +10707,14 @@ xlog_block_info(StringInfo buf, XLogReaderState *record)
 		RelFileNode rnode;
 		ForkNumber	forknum;
 		BlockNumber blk;
+		bool	blk_ref_exists;
 
-		if (!XLogRecHasBlockRef(record, block_id))
+		blk_ref_exists = XLogRecGetBlockTag(record, block_id, &rnode,
+											&forknum, &blk);
+
+		if (!blk_ref_exists)
 			continue;
 
-		XLogRecGetBlockTag(record, block_id, &rnode, &forknum, &blk);
 		if (forknum != MAIN_FORKNUM)
 			appendStringInfo(buf, "; blkref #%d: rel %u/%u/%u, fork %u, blk %u",
 							 block_id,
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 35029cf97d..821efd1ca1 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1533,7 +1533,7 @@ XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id,
 {
 	DecodedBkpBlock *bkpb;
 
-	if (!record->blocks[block_id].in_use)
+	if (!XLogRecHasBlockRef(record, block_id))
 		return false;
 
 	bkpb = &record->blocks[block_id];
@@ -1556,7 +1556,7 @@ XLogRecGetBlockData(XLogReaderState *record, uint8 block_id, Size *len)
 {
 	DecodedBkpBlock *bkpb;
 
-	if (!record->blocks[block_id].in_use)
+	if (!XLogRecHasBlockRef(record, block_id))
 		return NULL;
 
 	bkpb = &record->blocks[block_id];
@@ -1587,9 +1587,9 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page)
 	char	   *ptr;
 	PGAlignedBlock tmp;
 
-	if (!record->blocks[block_id].in_use)
+	if (!XLogRecHasBlockRef(record, block_id))
 		return false;
-	if (!record->blocks[block_id].has_image)
+	if (!XLogRecHasBlockImage(record, block_id))
 		return false;
 
 	bkpb = &record->blocks[block_id];
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index a6251e1a96..f5a554a390 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -513,10 +513,14 @@ XLogDumpDisplayRecord(XLogDumpConfig *config, XLogReaderState *record)
 		/* print block references (short format) */
 		for (block_id = 0; block_id <= record->max_block_id; block_id++)
 		{
-			if (!XLogRecHasBlockRef(record, block_id))
+			bool blk_ref_exists;
+
+			blk_ref_exists = XLogRecGetBlockTag(record, block_id, &rnode,
+												&forknum, &blk);
+
+			if (!blk_ref_exists)
 				continue;
 
-			XLogRecGetBlockTag(record, block_id, &rnode, &forknum, &blk);
 			if (forknum != MAIN_FORKNUM)
 				printf(", blkref #%d: rel %u/%u/%u fork %s blk %u",
 					   block_id,
@@ -544,10 +548,14 @@ XLogDumpDisplayRecord(XLogDumpConfig *config, XLogReaderState *record)
 		putchar('\n');
 		for (block_id = 0; block_id <= record->max_block_id; block_id++)
 		{
-			if (!XLogRecHasBlockRef(record, block_id))
+			bool    blk_ref_exists;
+
+			blk_ref_exists = XLogRecGetBlockTag(record, block_id, &rnode,
+												&forknum, &blk);
+
+			if (!blk_ref_exists)
 				continue;
 
-			XLogRecGetBlockTag(record, block_id, &rnode, &forknum, &blk);
 			printf("\tblkref #%d: rel %u/%u/%u fork %s blk %u",
 				   block_id,
 				   rnode.spcNode, rnode.dbNode, rnode.relNode,
-- 
2.25.1