Hi,

In a recent discussion [1], Michael Paquier asked if we can combine
pg_walinspect till_end_of_wal functions with other functions
pg_get_wal_records_info and pg_get_wal_stats. The code currently looks
much duplicated and the number of functions that pg_walinspect exposes
to the users is bloated. The point was that the till_end_of_wal
functions determine the end LSN and everything else that they do is
the same as their counterpart functions. Well, the idea then was to
keep things simple, not clutter the APIs, have better and consistent
user-inputted end_lsn validations at the cost of usability and code
redundancy. However, now I tend to agree with the feedback received.

I'm attaching a patch doing the $subject with the following behavior:
1. If start_lsn is NULL, error out/return NULL.
2. If end_lsn isn't specified, default to NULL, then determine the end_lsn.
3. If end_lsn is specified as NULL, then determine the end_lsn.
4. If end_lsn is specified as non-NULL, then determine if it is
greater than start_lsn if yes, go ahead do the job, otherwise error
out.

Another idea is to convert till_end_of_wal flavors to SQL-only
functions and remove the c code from pg_walinspect.c. However, I
prefer $subject and completely remove till_end_of_wal flavors for
better usability in the long term.

Thoughts?

[1] 
https://www.postgresql.org/message-id/CALj2ACV-WBN%3DEUgUPyYOGitp%2Brn163vMnQd%3DHcWrnKt-uqFYFA%40mail.gmail.com

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From 156e1d9148af79b905095b627e2db7919e8489ca Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Sun, 26 Feb 2023 13:34:33 +0000
Subject: [PATCH v1] Reduce pg_walinspect's functions without losing
 functionality

---
 contrib/pg_walinspect/Makefile                |   6 +-
 .../pg_walinspect/expected/oldextversions.out |  64 +++++++
 .../pg_walinspect/expected/pg_walinspect.out  |  32 +++-
 contrib/pg_walinspect/meson.build             |   4 +-
 .../pg_walinspect/pg_walinspect--1.1--1.2.sql |  78 ++++++++
 contrib/pg_walinspect/pg_walinspect.c         | 174 +++++++++---------
 contrib/pg_walinspect/pg_walinspect.control   |   2 +-
 contrib/pg_walinspect/sql/oldextversions.sql  |  27 +++
 contrib/pg_walinspect/sql/pg_walinspect.sql   |  28 ++-
 doc/src/sgml/pgwalinspect.sgml                |  54 ++----
 10 files changed, 336 insertions(+), 133 deletions(-)
 create mode 100644 contrib/pg_walinspect/expected/oldextversions.out
 create mode 100644 contrib/pg_walinspect/pg_walinspect--1.1--1.2.sql
 create mode 100644 contrib/pg_walinspect/sql/oldextversions.sql

diff --git a/contrib/pg_walinspect/Makefile b/contrib/pg_walinspect/Makefile
index 7033878a79..9527a0b268 100644
--- a/contrib/pg_walinspect/Makefile
+++ b/contrib/pg_walinspect/Makefile
@@ -7,9 +7,11 @@ OBJS = \
 PGFILEDESC = "pg_walinspect - functions to inspect contents of PostgreSQL Write-Ahead Log"
 
 EXTENSION = pg_walinspect
-DATA = pg_walinspect--1.0.sql pg_walinspect--1.0--1.1.sql
+DATA =  pg_walinspect--1.0.sql \
+		pg_walinspect--1.0--1.1.sql \
+		pg_walinspect--1.1--1.2.sql
 
-REGRESS = pg_walinspect
+REGRESS = pg_walinspect oldextversions
 
 REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_walinspect/walinspect.conf
 
diff --git a/contrib/pg_walinspect/expected/oldextversions.out b/contrib/pg_walinspect/expected/oldextversions.out
new file mode 100644
index 0000000000..65963c77f4
--- /dev/null
+++ b/contrib/pg_walinspect/expected/oldextversions.out
@@ -0,0 +1,64 @@
+-- test old extension version entry points
+DROP EXTENSION pg_walinspect;
+CREATE EXTENSION pg_walinspect VERSION '1.1';
+-- New function introduced in 1.1
+SELECT pg_get_functiondef('pg_get_wal_fpi_info'::regproc);
+                                                                                                          pg_get_functiondef                                                                                                           
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION public.pg_get_wal_fpi_info(start_lsn pg_lsn, end_lsn pg_lsn, OUT lsn pg_lsn, OUT reltablespace oid, OUT reldatabase oid, OUT relfilenode oid, OUT relblocknumber bigint, OUT forkname text, OUT fpi bytea)+
+  RETURNS SETOF record                                                                                                                                                                                                                +
+  LANGUAGE c                                                                                                                                                                                                                          +
+  PARALLEL SAFE STRICT                                                                                                                                                                                                                +
+ AS '$libdir/pg_walinspect', $function$pg_get_wal_fpi_info$function$                                                                                                                                                                  +
+ 
+(1 row)
+
+-- Move to new version 1.2
+ALTER EXTENSION pg_walinspect UPDATE TO '1.2';
+-- Redefined function
+SELECT pg_get_functiondef('pg_get_wal_records_info'::regproc);
+                                                                                                                                                                            pg_get_functiondef                                                                                                                                                                            
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION public.pg_get_wal_records_info(start_lsn pg_lsn, end_lsn pg_lsn DEFAULT NULL::pg_lsn, OUT start_lsn pg_lsn, OUT end_lsn pg_lsn, OUT prev_lsn pg_lsn, OUT xid xid, OUT resource_manager text, OUT record_type text, OUT record_length integer, OUT main_data_length integer, OUT fpi_length integer, OUT description text, OUT block_ref text)+
+  RETURNS SETOF record                                                                                                                                                                                                                                                                                                                                                   +
+  LANGUAGE c                                                                                                                                                                                                                                                                                                                                                             +
+  PARALLEL SAFE                                                                                                                                                                                                                                                                                                                                                          +
+ AS '$libdir/pg_walinspect', $function$pg_get_wal_records_info$function$                                                                                                                                                                                                                                                                                                 +
+ 
+(1 row)
+
+-- Removed function
+SELECT pg_get_functiondef('pg_get_wal_records_info_till_end_of_wal'::regproc);
+ERROR:  function "pg_get_wal_records_info_till_end_of_wal" does not exist
+LINE 1: SELECT pg_get_functiondef('pg_get_wal_records_info_till_end_...
+                                  ^
+-- Redefined function
+SELECT pg_get_functiondef('pg_get_wal_stats'::regproc);
+                                                                                                                                                                                                                     pg_get_functiondef                                                                                                                                                                                                                     
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION public.pg_get_wal_stats(start_lsn pg_lsn, end_lsn pg_lsn DEFAULT NULL::pg_lsn, per_record boolean DEFAULT false, OUT "resource_manager/record_type" text, OUT count bigint, OUT count_percentage double precision, OUT record_size bigint, OUT record_size_percentage double precision, OUT fpi_size bigint, OUT fpi_size_percentage double precision, OUT combined_size bigint, OUT combined_size_percentage double precision)+
+  RETURNS SETOF record                                                                                                                                                                                                                                                                                                                                                                                                                                     +
+  LANGUAGE c                                                                                                                                                                                                                                                                                                                                                                                                                                               +
+  PARALLEL SAFE                                                                                                                                                                                                                                                                                                                                                                                                                                            +
+ AS '$libdir/pg_walinspect', $function$pg_get_wal_stats$function$                                                                                                                                                                                                                                                                                                                                                                                          +
+ 
+(1 row)
+
+-- Removed function
+SELECT pg_get_functiondef('pg_get_wal_stats_till_end_of_wal'::regproc);
+ERROR:  function "pg_get_wal_stats_till_end_of_wal" does not exist
+LINE 1: SELECT pg_get_functiondef('pg_get_wal_stats_till_end_of_wal'...
+                                  ^
+-- Redefined function
+SELECT pg_get_functiondef('pg_get_wal_fpi_info'::regproc);
+                                                                                                                     pg_get_functiondef                                                                                                                     
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION public.pg_get_wal_fpi_info(start_lsn pg_lsn, end_lsn pg_lsn DEFAULT NULL::pg_lsn, OUT lsn pg_lsn, OUT reltablespace oid, OUT reldatabase oid, OUT relfilenode oid, OUT relblocknumber bigint, OUT forkname text, OUT fpi bytea)+
+  RETURNS SETOF record                                                                                                                                                                                                                                     +
+  LANGUAGE c                                                                                                                                                                                                                                               +
+  PARALLEL SAFE                                                                                                                                                                                                                                            +
+ AS '$libdir/pg_walinspect', $function$pg_get_wal_fpi_info$function$                                                                                                                                                                                       +
+ 
+(1 row)
+
+DROP EXTENSION pg_walinspect;
diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out
index 9bcb05354e..54284d22de 100644
--- a/contrib/pg_walinspect/expected/pg_walinspect.out
+++ b/contrib/pg_walinspect/expected/pg_walinspect.out
@@ -14,10 +14,30 @@ INSERT INTO sample_tbl SELECT * FROM generate_series(3, 4);
 -- ===================================================================
 -- Tests for input validation
 -- ===================================================================
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info('0/0'); -- ERROR
+ERROR:  WAL start LSN cannot be invalid
 SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn2', :'wal_lsn1'); -- ERROR
 ERROR:  WAL start LSN must be less than end LSN
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(NULL, NULL); -- ERROR
+ERROR:  WAL start LSN cannot be null
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', '0/0'); -- ERROR
+ERROR:  WAL end LSN cannot be invalid
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats('0/0'); -- ERROR
+ERROR:  WAL start LSN cannot be invalid
 SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1'); -- ERROR
 ERROR:  WAL start LSN must be less than end LSN
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(NULL, NULL); -- ERROR
+ERROR:  WAL start LSN cannot be null
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1', '0/0'); -- ERROR
+ERROR:  WAL end LSN cannot be invalid
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_fpi_info('0/0'); -- ERROR
+ERROR:  WAL start LSN cannot be invalid
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn2', :'wal_lsn1'); -- ERROR
+ERROR:  WAL start LSN must be less than end LSN
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_fpi_info(NULL, NULL); -- ERROR
+ERROR:  WAL start LSN cannot be null
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn1', '0/0'); -- ERROR
+ERROR:  WAL end LSN cannot be invalid
 -- ===================================================================
 -- Tests for all function executions
 -- ===================================================================
@@ -33,7 +53,7 @@ SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2'
  t
 (1 row)
 
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info_till_end_of_wal(:'wal_lsn1');
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1');
  ok 
 ----
  t
@@ -45,7 +65,7 @@ SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1', :'wal_lsn2');
  t
 (1 row)
 
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1');
  ok 
 ----
  t
@@ -90,6 +110,14 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn3', :'wal_lsn4')
  t
 (1 row)
 
+-- Check if we get FPI from WAL record.
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn3')
+  WHERE relfilenode = :'sample_tbl_oid';
+ ok 
+----
+ t
+(1 row)
+
 -- ===================================================================
 -- Tests for permissions
 -- ===================================================================
diff --git a/contrib/pg_walinspect/meson.build b/contrib/pg_walinspect/meson.build
index bf7b79b1b7..cecf041b5b 100644
--- a/contrib/pg_walinspect/meson.build
+++ b/contrib/pg_walinspect/meson.build
@@ -20,6 +20,7 @@ install_data(
   'pg_walinspect.control',
   'pg_walinspect--1.0.sql',
   'pg_walinspect--1.0--1.1.sql',
+  'pg_walinspect--1.1--1.2.sql',
   kwargs: contrib_data_args,
 )
 
@@ -30,10 +31,11 @@ tests += {
   'regress': {
     'sql': [
       'pg_walinspect',
+      'oldextversions',
     ],
     # Disabled because these tests require "wal_level=replica", which
     # some runningcheck users do not have (e.g. buildfarm clients).
     'regress_args': ['--temp-config', files('walinspect.conf')],
     'runningcheck': false,
   },
-}
+}
\ No newline at end of file
diff --git a/contrib/pg_walinspect/pg_walinspect--1.1--1.2.sql b/contrib/pg_walinspect/pg_walinspect--1.1--1.2.sql
new file mode 100644
index 0000000000..bd03ca4ba5
--- /dev/null
+++ b/contrib/pg_walinspect/pg_walinspect--1.1--1.2.sql
@@ -0,0 +1,78 @@
+/* contrib/pg_walinspect/pg_walinspect--1.1--1.2.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pg_walinspect UPDATE TO '1.2'" to load this file. \quit
+
+/* Drop unneeded functions and redefine needed ones */
+DROP FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn);
+DROP FUNCTION pg_get_wal_records_info_till_end_of_wal(pg_lsn);
+DROP FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean);
+DROP FUNCTION pg_get_wal_stats_till_end_of_wal(pg_lsn, boolean);
+DROP FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn);
+
+--
+-- pg_get_wal_records_info()
+--
+CREATE FUNCTION pg_get_wal_records_info(IN start_lsn pg_lsn,
+    IN end_lsn pg_lsn DEFAULT NULL,
+    OUT start_lsn pg_lsn,
+    OUT end_lsn pg_lsn,
+    OUT prev_lsn pg_lsn,
+    OUT xid xid,
+    OUT resource_manager text,
+    OUT record_type text,
+    OUT record_length int4,
+    OUT main_data_length int4,
+    OUT fpi_length int4,
+    OUT description text,
+    OUT block_ref text
+)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'pg_get_wal_records_info'
+LANGUAGE C CALLED ON NULL INPUT PARALLEL SAFE;
+
+REVOKE EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) FROM PUBLIC;
+GRANT EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) TO pg_read_server_files;
+
+--
+-- pg_get_wal_stats()
+--
+CREATE FUNCTION pg_get_wal_stats(IN start_lsn pg_lsn,
+    IN end_lsn pg_lsn DEFAULT NULL,
+    IN  per_record boolean DEFAULT false,
+    OUT "resource_manager/record_type" text,
+    OUT count int8,
+    OUT count_percentage float8,
+    OUT record_size int8,
+    OUT record_size_percentage float8,
+    OUT fpi_size int8,
+    OUT fpi_size_percentage float8,
+    OUT combined_size int8,
+    OUT combined_size_percentage float8
+)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'pg_get_wal_stats'
+LANGUAGE C CALLED ON NULL INPUT PARALLEL SAFE;
+
+REVOKE EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) FROM PUBLIC;
+GRANT EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) TO pg_read_server_files;
+
+--
+-- pg_get_wal_fpi_info()
+--
+CREATE FUNCTION pg_get_wal_fpi_info(IN start_lsn pg_lsn,
+	IN end_lsn pg_lsn DEFAULT NULL,
+	OUT lsn pg_lsn,
+	OUT reltablespace oid,
+	OUT reldatabase oid,
+	OUT relfilenode oid,
+	OUT relblocknumber int8,
+	OUT forkname text,
+	OUT fpi bytea
+)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'pg_get_wal_fpi_info'
+LANGUAGE C CALLED ON NULL INPUT PARALLEL SAFE;
+
+REVOKE EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) FROM PUBLIC;
+GRANT EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) TO pg_read_server_files;
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index b7b0a805ee..7d57a8b062 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -37,13 +37,13 @@ PG_FUNCTION_INFO_V1(pg_get_wal_records_info_till_end_of_wal);
 PG_FUNCTION_INFO_V1(pg_get_wal_stats);
 PG_FUNCTION_INFO_V1(pg_get_wal_stats_till_end_of_wal);
 
-static bool IsFutureLSN(XLogRecPtr lsn, XLogRecPtr *curr_lsn);
+static XLogRecPtr GetCurrentLSN(void);
 static XLogReaderState *InitXLogReaderState(XLogRecPtr lsn);
 static XLogRecord *ReadNextXLogRecord(XLogReaderState *xlogreader);
 static void GetWALRecordInfo(XLogReaderState *record, Datum *values,
 							 bool *nulls, uint32 ncols);
-static XLogRecPtr ValidateInputLSNs(bool till_end_of_wal,
-									XLogRecPtr start_lsn, XLogRecPtr end_lsn);
+static void ValidateInputLSNs(FunctionCallInfo fcinfo, XLogRecPtr *start_lsn,
+							  XLogRecPtr *end_lsn);
 static void GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
 							  XLogRecPtr end_lsn);
 static void GetXLogSummaryStats(XLogStats *stats, ReturnSetInfo *rsinfo,
@@ -59,27 +59,25 @@ static void GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
 static void GetWALFPIInfo(FunctionCallInfo fcinfo, XLogReaderState *record);
 
 /*
- * Check if the given LSN is in future. Also, return the LSN up to which the
- * server has WAL.
+ * Return the LSN up to which the server has WAL.
  */
-static bool
-IsFutureLSN(XLogRecPtr lsn, XLogRecPtr *curr_lsn)
+static XLogRecPtr
+GetCurrentLSN(void)
 {
+	XLogRecPtr	curr_lsn;
+
 	/*
 	 * We determine the current LSN of the server similar to how page_read
 	 * callback read_local_xlog_page_no_wait does.
 	 */
 	if (!RecoveryInProgress())
-		*curr_lsn = GetFlushRecPtr(NULL);
+		curr_lsn = GetFlushRecPtr(NULL);
 	else
-		*curr_lsn = GetXLogReplayRecPtr(NULL);
-
-	Assert(!XLogRecPtrIsInvalid(*curr_lsn));
+		curr_lsn = GetXLogReplayRecPtr(NULL);
 
-	if (lsn >= *curr_lsn)
-		return true;
+	Assert(!XLogRecPtrIsInvalid(curr_lsn));
 
-	return false;
+	return curr_lsn;
 }
 
 /*
@@ -295,6 +293,11 @@ GetWALFPIInfo(FunctionCallInfo fcinfo, XLogReaderState *record)
  * records between start and end LSNs.  Decompression is applied to the
  * blocks, if necessary.
  *
+ * This function determines end WAL LSN if it is not specified or specified as
+ * NULL. In such cases, the end WAL LSN is assigned with the last flushed WAL
+ * LSN when not in recovery or the last replayed WAL record LSN when in
+ * recovery.
+ *
  * This function emits an error if a future start or end WAL LSN i.e. WAL LSN
  * the database system doesn't know about is specified.
  */
@@ -307,11 +310,7 @@ pg_get_wal_fpi_info(PG_FUNCTION_ARGS)
 	MemoryContext old_cxt;
 	MemoryContext tmp_cxt;
 
-	start_lsn = PG_GETARG_LSN(0);
-	end_lsn = PG_GETARG_LSN(1);
-
-	end_lsn = ValidateInputLSNs(false, start_lsn, end_lsn);
-
+	ValidateInputLSNs(fcinfo, &start_lsn, &end_lsn);
 	InitMaterializedSRF(fcinfo, 0);
 
 	xlogreader = InitXLogReaderState(start_lsn);
@@ -363,7 +362,9 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
 
 	lsn = PG_GETARG_LSN(0);
 
-	if (IsFutureLSN(lsn, &curr_lsn))
+	curr_lsn = GetCurrentLSN();
+
+	if (lsn >= curr_lsn)
 	{
 		/*
 		 * GetFlushRecPtr or GetXLogReplayRecPtr gives "end+1" LSN of the last
@@ -402,44 +403,64 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
 }
 
 /*
- * Validate the input LSNs and compute end LSN for till_end_of_wal versions.
+ * Validate input LSNs and return start_lsn and end_lsn. Also, compute end LSN
+ * in case it is not specified.
  */
-static XLogRecPtr
-ValidateInputLSNs(bool till_end_of_wal, XLogRecPtr start_lsn,
-				  XLogRecPtr end_lsn)
+static void
+ValidateInputLSNs(FunctionCallInfo fcinfo, XLogRecPtr *start_lsn,
+				  XLogRecPtr *end_lsn)
 {
-	XLogRecPtr	curr_lsn;
+	XLogRecPtr	c_lsn;
+	XLogRecPtr  s_lsn;
+	XLogRecPtr	e_lsn;
 
-	if (IsFutureLSN(start_lsn, &curr_lsn))
-	{
-		/*
-		 * GetFlushRecPtr or GetXLogReplayRecPtr gives "end+1" LSN of the last
-		 * record flushed or replayed respectively. But let's use the LSN up
-		 * to "end" in user facing message.
-		 */
+	if (PG_ARGISNULL(0))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("cannot accept future start LSN"),
-				 errdetail("Last known WAL LSN on the database system is at %X/%X.",
-						   LSN_FORMAT_ARGS(curr_lsn))));
-	}
+				 errmsg("WAL start LSN cannot be null")));
 
-	if (till_end_of_wal)
-		end_lsn = curr_lsn;
+	s_lsn = PG_GETARG_LSN(0);
 
-	if (end_lsn > curr_lsn)
+	if (XLogRecPtrIsInvalid(s_lsn))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("cannot accept future end LSN"),
-				 errdetail("Last known WAL LSN on the database system is at %X/%X.",
-						   LSN_FORMAT_ARGS(curr_lsn))));
+				 errmsg("WAL start LSN cannot be invalid")));
 
-	if (start_lsn >= end_lsn)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("WAL start LSN must be less than end LSN")));
+	c_lsn = GetCurrentLSN();
+
+	if (PG_ARGISNULL(1))
+		e_lsn = c_lsn;
+	else
+	{
+		e_lsn = PG_GETARG_LSN(1);
+
+		if (XLogRecPtrIsInvalid(e_lsn))
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("WAL end LSN cannot be invalid")));
 
-	return end_lsn;
+		if (e_lsn > c_lsn)
+		{
+			/*
+			 * GetFlushRecPtr or GetXLogReplayRecPtr gives "end+1" LSN of the
+			 * last record flushed or replayed respectively. But let's use the
+			 * LSN up to "end" in user facing message.
+			 */
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("cannot accept future end LSN"),
+					 errdetail("Last known WAL LSN on the database system is at %X/%X.",
+							   LSN_FORMAT_ARGS(c_lsn))));
+		}
+
+		if (s_lsn >= e_lsn)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("WAL start LSN must be less than end LSN")));
+	}
+
+	*start_lsn = s_lsn;
+	*end_lsn = e_lsn;
 }
 
 /*
@@ -494,6 +515,11 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
 /*
  * Get info and data of all WAL records between start LSN and end LSN.
  *
+ * This function determines end WAL LSN if it is not specified or specified as
+ * NULL. In such cases, the end WAL LSN is assigned with the last flushed WAL
+ * LSN when not in recovery or the last replayed WAL record LSN when in
+ * recovery.
+ *
  * This function emits an error if a future start or end WAL LSN i.e. WAL LSN
  * the database system doesn't know about is specified.
  */
@@ -503,35 +529,23 @@ pg_get_wal_records_info(PG_FUNCTION_ARGS)
 	XLogRecPtr	start_lsn;
 	XLogRecPtr	end_lsn;
 
-	start_lsn = PG_GETARG_LSN(0);
-	end_lsn = PG_GETARG_LSN(1);
-
-	end_lsn = ValidateInputLSNs(false, start_lsn, end_lsn);
-
+	ValidateInputLSNs(fcinfo, &start_lsn, &end_lsn);
 	GetWALRecordsInfo(fcinfo, start_lsn, end_lsn);
 
 	PG_RETURN_VOID();
 }
 
 /*
- * Get info and data of all WAL records from start LSN till end of WAL.
+ * NB: This function does nothing and stays here for backward compatibility.
+ * Without it, the extension fails to install.
  *
- * This function emits an error if a future start i.e. WAL LSN the database
- * system doesn't know about is specified.
+ * Try using pg_get_wal_records_info() for the same till_end_of_wal
+ * functionaility.
  */
 Datum
 pg_get_wal_records_info_till_end_of_wal(PG_FUNCTION_ARGS)
 {
-	XLogRecPtr	start_lsn;
-	XLogRecPtr	end_lsn = InvalidXLogRecPtr;
-
-	start_lsn = PG_GETARG_LSN(0);
-
-	end_lsn = ValidateInputLSNs(true, start_lsn, end_lsn);
-
-	GetWALRecordsInfo(fcinfo, start_lsn, end_lsn);
-
-	PG_RETURN_VOID();
+	PG_RETURN_NULL();
 }
 
 /*
@@ -732,6 +746,11 @@ GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
 /*
  * Get stats of all WAL records between start LSN and end LSN.
  *
+ * This function determines end WAL LSN if it is not specified or specified as
+ * NULL. In such cases, the end WAL LSN is assigned with the last flushed WAL
+ * LSN when not in recovery or the last replayed WAL record LSN when in
+ * recovery.
+ *
  * This function emits an error if a future start or end WAL LSN i.e. WAL LSN
  * the database system doesn't know about is specified.
  */
@@ -742,36 +761,23 @@ pg_get_wal_stats(PG_FUNCTION_ARGS)
 	XLogRecPtr	end_lsn;
 	bool		stats_per_record;
 
-	start_lsn = PG_GETARG_LSN(0);
-	end_lsn = PG_GETARG_LSN(1);
+	ValidateInputLSNs(fcinfo, &start_lsn, &end_lsn);
 	stats_per_record = PG_GETARG_BOOL(2);
 
-	end_lsn = ValidateInputLSNs(false, start_lsn, end_lsn);
-
 	GetWalStats(fcinfo, start_lsn, end_lsn, stats_per_record);
 
 	PG_RETURN_VOID();
 }
 
 /*
- * Get stats of all WAL records from start LSN till end of WAL.
+ * NB: This function does nothing and stays here for backward compatibility.
+ * Without it, the extension fails to install.
  *
- * This function emits an error if a future start i.e. WAL LSN the database
- * system doesn't know about is specified.
+ * Try using pg_get_wal_records_info() for the same till_end_of_wal
+ * functionaility.
  */
 Datum
 pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS)
 {
-	XLogRecPtr	start_lsn;
-	XLogRecPtr	end_lsn = InvalidXLogRecPtr;
-	bool		stats_per_record;
-
-	start_lsn = PG_GETARG_LSN(0);
-	stats_per_record = PG_GETARG_BOOL(1);
-
-	end_lsn = ValidateInputLSNs(true, start_lsn, end_lsn);
-
-	GetWalStats(fcinfo, start_lsn, end_lsn, stats_per_record);
-
-	PG_RETURN_VOID();
+	PG_RETURN_NULL();
 }
diff --git a/contrib/pg_walinspect/pg_walinspect.control b/contrib/pg_walinspect/pg_walinspect.control
index efa3cb2cfe..5f574b865b 100644
--- a/contrib/pg_walinspect/pg_walinspect.control
+++ b/contrib/pg_walinspect/pg_walinspect.control
@@ -1,5 +1,5 @@
 # pg_walinspect extension
 comment = 'functions to inspect contents of PostgreSQL Write-Ahead Log'
-default_version = '1.1'
+default_version = '1.2'
 module_pathname = '$libdir/pg_walinspect'
 relocatable = true
diff --git a/contrib/pg_walinspect/sql/oldextversions.sql b/contrib/pg_walinspect/sql/oldextversions.sql
new file mode 100644
index 0000000000..ead4997d37
--- /dev/null
+++ b/contrib/pg_walinspect/sql/oldextversions.sql
@@ -0,0 +1,27 @@
+-- test old extension version entry points
+
+DROP EXTENSION pg_walinspect;
+CREATE EXTENSION pg_walinspect VERSION '1.1';
+
+-- New function introduced in 1.1
+SELECT pg_get_functiondef('pg_get_wal_fpi_info'::regproc);
+
+-- Move to new version 1.2
+ALTER EXTENSION pg_walinspect UPDATE TO '1.2';
+
+-- Redefined function
+SELECT pg_get_functiondef('pg_get_wal_records_info'::regproc);
+
+-- Removed function
+SELECT pg_get_functiondef('pg_get_wal_records_info_till_end_of_wal'::regproc);
+
+-- Redefined function
+SELECT pg_get_functiondef('pg_get_wal_stats'::regproc);
+
+-- Removed function
+SELECT pg_get_functiondef('pg_get_wal_stats_till_end_of_wal'::regproc);
+
+-- Redefined function
+SELECT pg_get_functiondef('pg_get_wal_fpi_info'::regproc);
+
+DROP EXTENSION pg_walinspect;
diff --git a/contrib/pg_walinspect/sql/pg_walinspect.sql b/contrib/pg_walinspect/sql/pg_walinspect.sql
index 849201a1f8..64fd95f387 100644
--- a/contrib/pg_walinspect/sql/pg_walinspect.sql
+++ b/contrib/pg_walinspect/sql/pg_walinspect.sql
@@ -17,10 +17,30 @@ INSERT INTO sample_tbl SELECT * FROM generate_series(3, 4);
 -- Tests for input validation
 -- ===================================================================
 
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info('0/0'); -- ERROR
+
 SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn2', :'wal_lsn1'); -- ERROR
 
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(NULL, NULL); -- ERROR
+
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', '0/0'); -- ERROR
+
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats('0/0'); -- ERROR
+
 SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1'); -- ERROR
 
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(NULL, NULL); -- ERROR
+
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1', '0/0'); -- ERROR
+
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_fpi_info('0/0'); -- ERROR
+
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn2', :'wal_lsn1'); -- ERROR
+
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_fpi_info(NULL, NULL); -- ERROR
+
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn1', '0/0'); -- ERROR
+
 -- ===================================================================
 -- Tests for all function executions
 -- ===================================================================
@@ -29,11 +49,11 @@ SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_record_info(:'wal_lsn1');
 
 SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2');
 
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info_till_end_of_wal(:'wal_lsn1');
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1');
 
 SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1', :'wal_lsn2');
 
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1');
 
 -- ===================================================================
 -- Test for filtering out WAL records of a particular table
@@ -68,6 +88,10 @@ SELECT pg_current_wal_lsn() AS wal_lsn4 \gset
 SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn3', :'wal_lsn4')
   WHERE relfilenode = :'sample_tbl_oid';
 
+-- Check if we get FPI from WAL record.
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn3')
+  WHERE relfilenode = :'sample_tbl_oid';
+
 -- ===================================================================
 -- Tests for permissions
 -- ===================================================================
diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml
index 22677e54f2..1b3626df6d 100644
--- a/doc/src/sgml/pgwalinspect.sgml
+++ b/doc/src/sgml/pgwalinspect.sgml
@@ -85,7 +85,7 @@ block_ref        | blkref #0: rel 1663/5/60221 fork main blk 2
     <varlistentry id="pgwalinspect-funcs-pg-get-wal-records-info">
     <term>
      <function>
-      pg_get_wal_records_info(start_lsn pg_lsn, end_lsn pg_lsn)
+      pg_get_wal_records_info(start_lsn pg_lsn, end_lsn pg_lsn DEFAULT NULL)
       returns setof record
      </function>
     </term>
@@ -94,9 +94,10 @@ block_ref        | blkref #0: rel 1663/5/60221 fork main blk 2
      <para>
       Gets information of all the valid WAL records between
       <replaceable>start_lsn</replaceable> and <replaceable>end_lsn</replaceable>.
-      Returns one row per WAL record. If <replaceable>start_lsn</replaceable>
-      or <replaceable>end_lsn</replaceable> are not yet available, the
-      function will raise an error. For example:
+      Returns one row per WAL record. If <replaceable>end_lsn</replaceable>
+      isn't specified, it returns information till the end of WAL.
+      If <replaceable>start_lsn</replaceable> or <replaceable>end_lsn</replaceable>
+      are not yet available, the function will raise an error. For example:
 <screen>
 postgres=# SELECT * FROM pg_get_wal_records_info('0/1E913618', '0/1E913740') LIMIT 1;
 -[ RECORD 1 ]----+--------------------------------------------------------------
@@ -116,27 +117,10 @@ block_ref        |
     </listitem>
    </varlistentry>
 
-    <varlistentry id="pgwalinspect-funcs-pg-get-wal-records-info-till-end-of-wal">
-    <term>
-     <function>
-      pg_get_wal_records_info_till_end_of_wal(start_lsn pg_lsn)
-      returns setof record
-     </function>
-    </term>
-
-    <listitem>
-     <para>
-      This function is the same as <function>pg_get_wal_records_info()</function>,
-      except that it gets information of all the valid WAL records from
-      <replaceable>start_lsn</replaceable> till the end of WAL.
-     </para>
-    </listitem>
-   </varlistentry>
-
     <varlistentry id="pgwalinspect-funcs-pg-get-wal-stats">
     <term>
      <function>
-      pg_get_wal_stats(start_lsn pg_lsn, end_lsn pg_lsn, per_record boolean DEFAULT false)
+      pg_get_wal_stats(start_lsn pg_lsn, end_lsn pg_lsn DEFAULT NULL, per_record boolean DEFAULT false)
       returns setof record
      </function>
     </term>
@@ -149,7 +133,8 @@ block_ref        |
       <replaceable>resource_manager</replaceable> type. When
       <replaceable>per_record</replaceable> is set to <literal>true</literal>,
       it returns one row per <replaceable>record_type</replaceable>.
-      If <replaceable>start_lsn</replaceable>
+      If <replaceable>end_lsn</replaceable> isn't specified, it returns
+      information till the end of WAL. If <replaceable>start_lsn</replaceable>
       or <replaceable>end_lsn</replaceable> are not yet available, the
       function will raise an error. For example:
 <screen>
@@ -171,28 +156,14 @@ combined_size_percentage     | 2.8634072910530795
     </listitem>
    </varlistentry>
 
-    <varlistentry id="pgwalinspect-funcs-pg-get-wal-stats-till-end-of-wal">
+    <varlistentry>
     <term>
      <function>
-      pg_get_wal_stats_till_end_of_wal(start_lsn pg_lsn, end_lsn pg_lsn, per_record boolean DEFAULT false)
+      pg_get_wal_fpi_info(start_lsn pg_lsn, end_lsn pg_lsn DEFAULT NULL)
       returns setof record
      </function>
     </term>
 
-    <listitem>
-     <para>
-      This function is the same as <function>pg_get_wal_stats()</function>,
-      except that it gets statistics of all the valid WAL records from
-      <replaceable>start_lsn</replaceable> till end of WAL.
-     </para>
-    </listitem>
-   </varlistentry>
-
-   <varlistentry>
-    <term>
-     <function>pg_get_wal_fpi_info(start_lsn pg_lsn, end_lsn pg_lsn) returns setof record</function>
-    </term>
-
     <listitem>
      <para>
       Gets a copy of full page images as <type>bytea</type> values (after
@@ -200,8 +171,9 @@ combined_size_percentage     | 2.8634072910530795
       with all the valid WAL records between
       <replaceable>start_lsn</replaceable> and
       <replaceable>end_lsn</replaceable>. Returns one row per full page image.
-      If <replaceable>start_lsn</replaceable> or
-      <replaceable>end_lsn</replaceable> are not yet available, the function
+      If <replaceable>end_lsn</replaceable> isn't specified, it returns
+      information till the end of WAL. If <replaceable>start_lsn</replaceable>
+      or <replaceable>end_lsn</replaceable> are not yet available, the function
       will raise an error. For example:
 <screen>
 postgres=# SELECT lsn, reltablespace, reldatabase, relfilenode, relblocknumber,
-- 
2.34.1

Reply via email to