v1-PG15-Use-correct-LSN-for-error-reporting-in-pg_walinsp.patch

From edc92468032e2f491131639aef45e991ebde3c8e Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Wed, 17 Aug 2022 04:33:36 +0000
Subject: [PATCH v1] Use correct LSN for error reporting in pg_walinspect

Usage of ReadNextXLogRecord()'s first_record parameter for error
reporting isn't always correct. For instance, in GetWALRecordsInfo()
and GetWalStats(), we're reading multiple records, and first_record
is always passed as the LSN of the first record which is then used
for error reporting for later WAL record read failures. This isn't
correct.

The correct parameter to use for error reports in case of WAL
reading failures is xlogreader->EndRecPtr. This change fixes it.

While on it, removed an unnecessary Assert in pg_walinspect code.

Reported-by: Robert Haas
Author: Bharath Rupireddy
Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/CA%2BTgmoZAOGzPUifrcZRjFZ2vbtcw3mp-mN6UgEoEcQg6bY3OVg%40mail.gmail.com
Backpatch-through: 15
---
 contrib/pg_walinspect/pg_walinspect.c | 33 +++++++++++++--------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index a082dfb331..9f2c8d00a6 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -39,8 +39,7 @@ PG_FUNCTION_INFO_V1(pg_get_wal_stats_till_end_of_wal);
 static bool IsFutureLSN(XLogRecPtr lsn, XLogRecPtr *curr_lsn);
 static XLogReaderState *InitXLogReaderState(XLogRecPtr lsn,
 											XLogRecPtr *first_record);
-static XLogRecord *ReadNextXLogRecord(XLogReaderState *xlogreader,
-									  XLogRecPtr first_record);
+static XLogRecord *ReadNextXLogRecord(XLogReaderState *xlogreader);
 static void GetWALRecordInfo(XLogReaderState *record, XLogRecPtr lsn,
 							 Datum *values, bool *nulls, uint32 ncols);
 static XLogRecPtr ValidateInputLSNs(bool till_end_of_wal,
@@ -90,6 +89,7 @@ InitXLogReaderState(XLogRecPtr lsn, XLogRecPtr *first_record)
 {
 	XLogReaderState *xlogreader;
 	ReadLocalXLogPageNoWaitPrivate *private_data;
+	XLogRecPtr	first_valid_record;
 
 	/*
 	 * Reading WAL below the first page of the first segments isn't allowed.
@@ -117,13 +117,16 @@ InitXLogReaderState(XLogRecPtr lsn, XLogRecPtr *first_record)
 				 errdetail("Failed while allocating a WAL reading processor.")));
 
 	/* first find a valid recptr to start from */
-	*first_record = XLogFindNextRecord(xlogreader, lsn);
+	first_valid_record = XLogFindNextRecord(xlogreader, lsn);
 
-	if (XLogRecPtrIsInvalid(*first_record))
+	if (XLogRecPtrIsInvalid(first_valid_record))
 		ereport(ERROR,
 				(errmsg("could not find a valid record after %X/%X",
 						LSN_FORMAT_ARGS(lsn))));
 
+	if (first_record != NULL)
+		*first_record = first_valid_record;
+
 	return xlogreader;
 }
 
@@ -140,7 +143,7 @@ InitXLogReaderState(XLogRecPtr lsn, XLogRecPtr *first_record)
  * that case we'll return NULL.
  */
 static XLogRecord *
-ReadNextXLogRecord(XLogReaderState *xlogreader, XLogRecPtr first_record)
+ReadNextXLogRecord(XLogReaderState *xlogreader)
 {
 	XLogRecord *record;
 	char	   *errormsg;
@@ -162,12 +165,12 @@ ReadNextXLogRecord(XLogReaderState *xlogreader, XLogRecPtr first_record)
 			ereport(ERROR,
 					(errcode_for_file_access(),
 					 errmsg("could not read WAL at %X/%X: %s",
-							LSN_FORMAT_ARGS(first_record), errormsg)));
+							LSN_FORMAT_ARGS(xlogreader->EndRecPtr), errormsg)));
 		else
 			ereport(ERROR,
 					(errcode_for_file_access(),
 					 errmsg("could not read WAL at %X/%X",
-							LSN_FORMAT_ARGS(first_record))));
+							LSN_FORMAT_ARGS(xlogreader->EndRecPtr))));
 	}
 
 	return record;
@@ -260,11 +263,11 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
 
 	xlogreader = InitXLogReaderState(lsn, &first_record);
 
-	if (!ReadNextXLogRecord(xlogreader, first_record))
+	if (!ReadNextXLogRecord(xlogreader))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 				 errmsg("could not read WAL at %X/%X",
-						LSN_FORMAT_ARGS(first_record))));
+						LSN_FORMAT_ARGS(xlogreader->EndRecPtr))));
 
 	MemSet(values, 0, sizeof(values));
 	MemSet(nulls, 0, sizeof(nulls));
@@ -331,7 +334,6 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
 				  XLogRecPtr end_lsn)
 {
 #define PG_GET_WAL_RECORDS_INFO_COLS 11
-	XLogRecPtr	first_record;
 	XLogReaderState *xlogreader;
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
 	Datum		values[PG_GET_WAL_RECORDS_INFO_COLS];
@@ -339,14 +341,12 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
 
 	SetSingleFuncCall(fcinfo, 0);
 
-	xlogreader = InitXLogReaderState(start_lsn, &first_record);
-
-	Assert(xlogreader);
+	xlogreader = InitXLogReaderState(start_lsn, NULL);
 
 	MemSet(values, 0, sizeof(values));
 	MemSet(nulls, 0, sizeof(nulls));
 
-	while (ReadNextXLogRecord(xlogreader, first_record) &&
+	while (ReadNextXLogRecord(xlogreader) &&
 		   xlogreader->EndRecPtr <= end_lsn)
 	{
 		GetWALRecordInfo(xlogreader, xlogreader->currRecPtr, values, nulls,
@@ -554,7 +554,6 @@ GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
 			XLogRecPtr end_lsn, bool stats_per_record)
 {
 #define PG_GET_WAL_STATS_COLS 9
-	XLogRecPtr	first_record;
 	XLogReaderState *xlogreader;
 	XLogStats	stats;
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
@@ -563,11 +562,11 @@ GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
 
 	SetSingleFuncCall(fcinfo, 0);
 
-	xlogreader = InitXLogReaderState(start_lsn, &first_record);
+	xlogreader = InitXLogReaderState(start_lsn, NULL);
 
 	MemSet(&stats, 0, sizeof(stats));
 
-	while (ReadNextXLogRecord(xlogreader, first_record) &&
+	while (ReadNextXLogRecord(xlogreader) &&
 		   xlogreader->EndRecPtr <= end_lsn)
 	{
 		XLogRecStoreStats(&stats, xlogreader);
-- 
2.34.1