Make XLogRecGetBlockTag() throw error if there's no such block.

All but a few existing callers assume without checking that this
function succeeds.  While it probably will, that's a poor excuse for
not checking.  Let's make it return void and instead throw an error
if it doesn't find the block reference.  Callers that actually need
to handle the no-such-block case must now use the underlying function
XLogRecGetBlockTagExtended.

In addition to being a bit less error-prone, this should also serve
to suppress some Coverity complaints about XLogRecGetBlockRefInfo.

While at it, clean up some inconsistency about use of the
XLogRecHasBlockRef macro: make XLogRecGetBlockTagExtended use
that instead of open-coding the same condition, and avoid calling
XLogRecHasBlockRef twice in relevant code paths.  (That is,
calling XLogRecHasBlockRef followed by XLogRecGetBlockTag is now
deprecated: use XLogRecGetBlockTagExtended instead.)

Patch HEAD only; this doesn't seem to have enough value to consider
a back-branch API break.

Discussion: https://postgr.es/m/425039.1649701221@sss.pgh.pa.us

1 files changed, 18 insertions, 6 deletions

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index b3e37003ac5..cf5db23cb86 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -37,6 +37,8 @@
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "utils/memutils.h"
+#else
+#include "common/logging.h"
 #endif
 
 static void report_invalid_record(XLogReaderState *state, const char *fmt,...)
@@ -1918,14 +1920,25 @@ err:
 
 /*
  * Returns information about the block that a block reference refers to.
- * See XLogRecGetBlockTagExtended().
+ *
+ * This is like XLogRecGetBlockTagExtended, except that the block reference
+ * must exist and there's no access to prefetch_buffer.
  */
-bool
+void
 XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id,
 				   RelFileNode *rnode, ForkNumber *forknum, BlockNumber *blknum)
 {
-	return XLogRecGetBlockTagExtended(record, block_id, rnode, forknum, blknum,
-									  NULL);
+	if (!XLogRecGetBlockTagExtended(record, block_id, rnode, forknum, blknum,
+									NULL))
+	{
+#ifndef FRONTEND
+		elog(ERROR, "failed to locate backup block with ID %d in WAL record",
+			 block_id);
+#else
+		pg_fatal("failed to locate backup block with ID %d in WAL record",
+				 block_id);
+#endif
+	}
 }
 
 /*
@@ -1944,8 +1957,7 @@ XLogRecGetBlockTagExtended(XLogReaderState *record, uint8 block_id,
 {
 	DecodedBkpBlock *bkpb;
 
-	if (block_id > record->record->max_block_id ||
-		!record->record->blocks[block_id].in_use)
+	if (!XLogRecHasBlockRef(record, block_id))
 		return false;
 
 	bkpb = &record->record->blocks[block_id];