On Fri, Mar 10, 2023 at 04:45:06PM +0530, Bharath Rupireddy wrote: > Any comments on the attached v5 patch?
I have reviewed the patch, and found it pretty messy. The tests
should have been divided into their own patch, I think. This is
rather straight-forward once the six functions have their checks
grouped together. The result was pretty good, so I have begun by
applying that as 1f282c2. This also includes most of the refinements
you have proposed for the whitespaces in the tests. Note that we were
missing a few spots with the bound checks for the six functions, so
now the coverage should be full.
After that comes the rest of the patch, and I have found a couple of
mistakes.
- 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
[...]
- 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)
This part of the documentation is now incorrect.
+-- Make sure checkpoints don't interfere with the test.
+SELECT 'init' FROM
pg_create_physical_replication_slot('regress_pg_walinspect_slot', true, false);
Adding a physical slot is better for stability of course, but the test
also has to drop it or installcheck would cause an existing cluster to
have that still around. The third argument could be true, as well, so
as we'd use a temporary slot.
- If <replaceable>start_lsn</replaceable>
- or <replaceable>end_lsn</replaceable> are not yet available, the
- function will raise an error. For example:
+ If a future <replaceable>end_lsn</replaceable> (i.e. the LSN server
+ doesn't know about) is specified, it returns stats till end of WAL. It
+ will raise an error, if the server doesn't have WAL available at given
+ <replaceable>start_lsn</replaceable> or if the
+ <replaceable>start_lsn</replaceable> is in future or is past the
+ <replaceable>end_lsn</replaceable>. For example, usage of the function is
+ as follows:
Hmm. I would simplify that, and just mention that an error is raised
when the start LSN is available, without caring about the rest (valid
end LSN being past the current insert LSN, and error if start > end,
the second being obvious).
+ <note>
+ <para>
+ Note that <function>pg_get_wal_records_info_till_end_of_wal</function> and
+ <function>pg_get_wal_stats_till_end_of_wal</function> functions have been
+ removed in the <filename>pg_walinspect</filename> version
+ <literal>1.1</literal>. The same functionality can be achieved with
+ <function>pg_get_wal_records_info</function> and
+ <function>pg_get_wal_stats</function> functions by specifying a future
+ <replaceable>end_lsn</replaceable>. However,
<function>till_end_of_wal</function>
+ functions will still work if the extension is installed explicitly with
+ version <literal>1.0</literal>.
+ </para>
+ </note>
Not convinced that this is necessary.
+ GetInputLSNs(fcinfo, &start_lsn, &end_lsn, till_end_of_wal);
+
+ stats_per_record = PG_GETARG_BOOL(2);
This code in GetWalStats() is incorrect.
pg_get_wal_stats_till_end_of_wal() has a stats_per_record, but as
*second* argument, so this would be broken.
+ GetInputLSNs(fcinfo, &start_lsn, &end_lsn, till_end_of_wal);
Coming from the last point, I think that this interface is confusing,
and actually incorrect. From what I can see, we should be doing what
~15 has by grepping the argument values within the main function
calls, and just pass them down to the internal routines GetWalStats()
and GetWALRecordsInfo().
-static bool
-IsFutureLSN(XLogRecPtr lsn, XLogRecPtr *curr_lsn)
+static XLogRecPtr
+GetCurrentLSN(void)
This wrapper is actually a good idea.
At the end, I am finishing with the attached. ValidateInputLSNs()
ought to be called, IMO, when the caller of the SQL functions can
directly specify an end_lsn. This means that there is no point to do
this check in the two till_end_* functions. This has as cost two
extra checks to make sure that the start_lsn is not higher than the
current LSN, but I am fine to live with that. It seemed rather
natural to me to let ValidateInputLSNs() do a refresh of the end_lsn
if it sees that it is higher than the current LSN. And if you look
closely, you will see that we only call *once* GetCurrentLSN() for
each function call, so the maths are more precise.
I have cleaned up the comments of the modules, while on it, as there
was not much value in copy-pasting how a function fails while there is
a centralized validation code path. The tests for the till_end()
functions have been moved to the test path where we install 1.0.
With all these cleanups done, there is less code than at the
beginning, which comes from the docs, so the current code does not
change in size:
7 files changed, 173 insertions(+), 206 deletions(-)
--
Michael
From 74b873e5d61173719ec1961adbc30d711b63f79b Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 13 Mar 2023 15:51:51 +0900
Subject: [PATCH v6] Rework pg_walinspect functions
---
doc/src/sgml/pgwalinspect.sgml | 53 +----
.../pg_walinspect/expected/oldextversions.out | 45 +++-
.../pg_walinspect/expected/pg_walinspect.out | 42 ++--
.../pg_walinspect/pg_walinspect--1.0--1.1.sql | 4 +
contrib/pg_walinspect/pg_walinspect.c | 211 +++++++-----------
contrib/pg_walinspect/sql/oldextversions.sql | 18 ++
contrib/pg_walinspect/sql/pg_walinspect.sql | 6 +-
7 files changed, 173 insertions(+), 206 deletions(-)
diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml
index 3b19863dce..4f7b8f8f19 100644
--- a/doc/src/sgml/pgwalinspect.sgml
+++ b/doc/src/sgml/pgwalinspect.sgml
@@ -94,9 +94,9 @@ 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. The function raises an error if
+ <replaceable>start_lsn</replaceable> is not available. For example, usage
+ of the function is as follows:
<screen>
postgres=# SELECT * FROM pg_get_wal_records_info('0/1E913618', '0/1E913740') LIMIT 1;
-[ RECORD 1 ]----+--------------------------------------------------------------
@@ -116,23 +116,6 @@ 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>
@@ -148,10 +131,9 @@ block_ref |
<replaceable>end_lsn</replaceable>. By default, it returns one row per
<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>
- or <replaceable>end_lsn</replaceable> are not yet available, the
- function will raise an error. For example:
+ it returns one row per <replaceable>record_type</replaceable>. An error
+ is raised if <replaceable>start_lsn</replaceable> is not available. For
+ example, usage of the function is as follows:
<screen>
postgres=# SELECT * FROM pg_get_wal_stats('0/1E847D00', '0/1E84F500')
WHERE count > 0 LIMIT 1 AND
@@ -171,23 +153,6 @@ combined_size_percentage | 2.8634072910530795
</listitem>
</varlistentry>
- <varlistentry id="pgwalinspect-funcs-pg-get-wal-stats-till-end-of-wal">
- <term>
- <function>
- pg_get_wal_stats_till_end_of_wal(start_lsn pg_lsn, per_record boolean DEFAULT false)
- 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_block_info(start_lsn pg_lsn, end_lsn pg_lsn) returns setof record</function>
@@ -202,9 +167,9 @@ combined_size_percentage | 2.8634072910530795
and their information associated with all the valid WAL records between
<replaceable>start_lsn</replaceable> and
<replaceable>end_lsn</replaceable>. Returns one row per block registered
- in a WAL record. If <replaceable>start_lsn</replaceable> or
- <replaceable>end_lsn</replaceable> are not yet available, the function
- will raise an error. For example:
+ in a WAL record. An error is raised if
+ <replaceable>start_lsn</replaceable> is not available. For example, usage
+ of the function is as follows:
<screen>
postgres=# SELECT lsn, blockid, reltablespace, reldatabase, relfilenode,
relblocknumber, forkname,
diff --git a/contrib/pg_walinspect/expected/oldextversions.out b/contrib/pg_walinspect/expected/oldextversions.out
index 0db3538693..c35626cd3c 100644
--- a/contrib/pg_walinspect/expected/oldextversions.out
+++ b/contrib/pg_walinspect/expected/oldextversions.out
@@ -1,5 +1,7 @@
-- test old extension version entry points
CREATE EXTENSION pg_walinspect WITH VERSION '1.0';
+-- Mask DETAIL messages as these could refer to current LSN positions.
+\set VERBOSITY terse
-- List what version 1.0 contains
\dx+ pg_walinspect
Objects in extension "pg_walinspect"
@@ -12,19 +14,50 @@ CREATE EXTENSION pg_walinspect WITH VERSION '1.0';
function pg_get_wal_stats_till_end_of_wal(pg_lsn,boolean)
(5 rows)
+-- Make sure checkpoints don't interfere with the test.
+SELECT 'init' FROM pg_create_physical_replication_slot('regress_pg_walinspect_slot', true, false);
+ ?column?
+----------
+ init
+(1 row)
+
+CREATE TABLE sample_tbl(col1 int, col2 int);
+SELECT pg_current_wal_lsn() AS wal_lsn1 \gset
+INSERT INTO sample_tbl SELECT * FROM generate_series(1, 2);
+-- Check bounds for these past functions.
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info_till_end_of_wal(:'wal_lsn1');
+ ok
+----
+ t
+(1 row)
+
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
+ ok
+----
+ t
+(1 row)
+
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
+ERROR: WAL start LSN must be less than current LSN
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
+ERROR: WAL start LSN must be less than current LSN
-- Move to new version 1.1
ALTER EXTENSION pg_walinspect UPDATE TO '1.1';
-- List what version 1.1 contains
\dx+ pg_walinspect
- Objects in extension "pg_walinspect"
- Object description
------------------------------------------------------------
+ Objects in extension "pg_walinspect"
+ Object description
+--------------------------------------------------
function pg_get_wal_block_info(pg_lsn,pg_lsn)
function pg_get_wal_record_info(pg_lsn)
function pg_get_wal_records_info(pg_lsn,pg_lsn)
- function pg_get_wal_records_info_till_end_of_wal(pg_lsn)
function pg_get_wal_stats(pg_lsn,pg_lsn,boolean)
- function pg_get_wal_stats_till_end_of_wal(pg_lsn,boolean)
-(6 rows)
+(4 rows)
+
+SELECT pg_drop_replication_slot('regress_pg_walinspect_slot');
+ pg_drop_replication_slot
+--------------------------
+
+(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 c9fb781055..1ab6f6556d 100644
--- a/contrib/pg_walinspect/expected/pg_walinspect.out
+++ b/contrib/pg_walinspect/expected/pg_walinspect.out
@@ -37,24 +37,32 @@ ERROR: WAL start LSN must be less than end LSN
-- LSNs with the highest value possible.
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
ERROR: cannot accept future input LSN
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future start LSN
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future start LSN
--- failures with end LSNs
+-- Success with end LSNs.
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future end LSN
+ ok
+----
+ t
+(1 row)
+
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future end LSN
+ ok
+----
+ t
+(1 row)
+
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future end LSN
+ ok
+----
+ t
+(1 row)
+
-- failures with start LSNs
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future start LSN
+ERROR: WAL start LSN must be less than current LSN
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future start LSN
+ERROR: WAL start LSN must be less than current LSN
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future start LSN
+ERROR: WAL start LSN must be less than current LSN
-- ===================================================================
-- Tests for all function executions
-- ===================================================================
@@ -64,18 +72,6 @@ SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_record_info(:'wal_lsn1');
t
(1 row)
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info_till_end_of_wal(:'wal_lsn1');
- ok
-----
- t
-(1 row)
-
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
- ok
-----
- t
-(1 row)
-
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2');
ok
----
diff --git a/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql b/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql
index e674ef25aa..586c3b4467 100644
--- a/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql
+++ b/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql
@@ -3,6 +3,10 @@
-- complain if script is sourced in psql, rather than via ALTER EXTENSION
\echo Use "ALTER EXTENSION pg_walinspect UPDATE TO '1.1'" to load this file. \quit
+-- Unsupported functions after 1.1.
+DROP FUNCTION pg_get_wal_records_info_till_end_of_wal(pg_lsn);
+DROP FUNCTION pg_get_wal_stats_till_end_of_wal(pg_lsn, boolean);
+
--
-- pg_get_wal_block_info()
--
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index ee88dc4992..7ab28458c4 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -38,14 +38,14 @@ 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 void ValidateInputLSNs(XLogRecPtr start_lsn, XLogRecPtr *end_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 GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
+static void GetWALRecordsInfo(FunctionCallInfo fcinfo,
+ XLogRecPtr start_lsn,
XLogRecPtr end_lsn);
static void GetXLogSummaryStats(XLogStats *stats, ReturnSetInfo *rsinfo,
Datum *values, bool *nulls, uint32 ncols,
@@ -55,32 +55,32 @@ static void FillXLogStatsRow(const char *name, uint64 n, uint64 total_count,
uint64 fpi_len, uint64 total_fpi_len,
uint64 tot_len, uint64 total_len,
Datum *values, bool *nulls, uint32 ncols);
-static void GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
- XLogRecPtr end_lsn, bool stats_per_record);
+static void GetWalStats(FunctionCallInfo fcinfo,
+ XLogRecPtr start_lsn,
+ XLogRecPtr end_lsn,
+ bool stats_per_record);
static void GetWALBlockInfo(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);
+ curr_lsn = GetXLogReplayRecPtr(NULL);
- Assert(!XLogRecPtrIsInvalid(*curr_lsn));
+ Assert(!XLogRecPtrIsInvalid(curr_lsn));
- if (lsn >= *curr_lsn)
- return true;
-
- return false;
+ return curr_lsn;
}
/*
@@ -354,23 +354,17 @@ GetWALBlockInfo(FunctionCallInfo fcinfo, XLogReaderState *record)
* their relation information, and the data saved in each block associated
* to a record. Decompression is applied to the full page images, if
* necessary.
- *
- * 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.
*/
Datum
pg_get_wal_block_info(PG_FUNCTION_ARGS)
{
- XLogRecPtr start_lsn;
- XLogRecPtr end_lsn;
+ XLogRecPtr start_lsn = PG_GETARG_LSN(0);
+ XLogRecPtr end_lsn = PG_GETARG_LSN(1);
XLogReaderState *xlogreader;
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(start_lsn, &end_lsn);
InitMaterializedSRF(fcinfo, 0);
@@ -404,9 +398,6 @@ pg_get_wal_block_info(PG_FUNCTION_ARGS)
/*
* Get WAL record info.
- *
- * This function emits an error if a future WAL LSN i.e. WAL LSN the database
- * system doesn't know about is specified.
*/
Datum
pg_get_wal_record_info(PG_FUNCTION_ARGS)
@@ -422,20 +413,14 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
HeapTuple tuple;
lsn = PG_GETARG_LSN(0);
+ curr_lsn = GetCurrentLSN();
- if (IsFutureLSN(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 (lsn >= curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("cannot accept future input LSN"),
errdetail("Last known WAL LSN on the database system is at %X/%X.",
LSN_FORMAT_ARGS(curr_lsn))));
- }
/* Build a tuple descriptor for our result type. */
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
@@ -462,44 +447,28 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
}
/*
- * Validate the input LSNs and compute end LSN for till_end_of_wal versions.
+ * Validate start and end LSNs coming from the function inputs.
+ *
+ * If end_lsn is found to be higher than the current LSN reported by the
+ * cluster, use the current LSN as the upper bound.
*/
-static XLogRecPtr
-ValidateInputLSNs(bool till_end_of_wal, XLogRecPtr start_lsn,
- XLogRecPtr end_lsn)
+static void
+ValidateInputLSNs(XLogRecPtr start_lsn, XLogRecPtr *end_lsn)
{
- XLogRecPtr curr_lsn;
+ XLogRecPtr curr_lsn = GetCurrentLSN();
- 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 (start_lsn > curr_lsn)
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 must be less than current LSN")));
- if (till_end_of_wal)
- end_lsn = curr_lsn;
-
- if (end_lsn > curr_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))));
-
- if (start_lsn >= end_lsn)
+ if (start_lsn > *end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("WAL start LSN must be less than end LSN")));
- return end_lsn;
+ if (*end_lsn > curr_lsn)
+ *end_lsn = curr_lsn;
}
/*
@@ -517,6 +486,8 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
MemoryContext old_cxt;
MemoryContext tmp_cxt;
+ Assert(start_lsn <= end_lsn);
+
InitMaterializedSRF(fcinfo, 0);
xlogreader = InitXLogReaderState(start_lsn);
@@ -552,43 +523,17 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
}
/*
- * Get info and data of all WAL records between start LSN and end LSN.
+ * pg_get_wal_records_info
*
- * 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.
+ * Get info and data of all WAL records between start LSN and end LSN.
*/
Datum
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);
-
- GetWALRecordsInfo(fcinfo, start_lsn, end_lsn);
-
- PG_RETURN_VOID();
-}
-
-/*
- * Get info and data of all WAL records from start LSN till end of WAL.
- *
- * This function emits an error if a future start i.e. WAL LSN the database
- * system doesn't know about is specified.
- */
-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);
+ XLogRecPtr start_lsn = PG_GETARG_LSN(0);
+ XLogRecPtr end_lsn = PG_GETARG_LSN(1);
+ ValidateInputLSNs(start_lsn, &end_lsn);
GetWALRecordsInfo(fcinfo, start_lsn, end_lsn);
PG_RETURN_VOID();
@@ -648,13 +593,13 @@ GetXLogSummaryStats(XLogStats *stats, ReturnSetInfo *rsinfo,
Datum *values, bool *nulls, uint32 ncols,
bool stats_per_record)
{
- MemoryContext old_cxt;
- MemoryContext tmp_cxt;
- uint64 total_count = 0;
- uint64 total_rec_len = 0;
- uint64 total_fpi_len = 0;
- uint64 total_len = 0;
- int ri;
+ MemoryContext old_cxt;
+ MemoryContext tmp_cxt;
+ uint64 total_count = 0;
+ uint64 total_rec_len = 0;
+ uint64 total_fpi_len = 0;
+ uint64 total_len = 0;
+ int ri;
/*
* Each row shows its percentages of the total, so make a first pass to
@@ -757,8 +702,8 @@ GetXLogSummaryStats(XLogStats *stats, ReturnSetInfo *rsinfo,
* Get WAL stats between start LSN and end LSN.
*/
static void
-GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
- XLogRecPtr end_lsn, bool stats_per_record)
+GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn, XLogRecPtr end_lsn,
+ bool stats_per_record)
{
#define PG_GET_WAL_STATS_COLS 9
XLogReaderState *xlogreader;
@@ -767,6 +712,8 @@ GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
Datum values[PG_GET_WAL_STATS_COLS] = {0};
bool nulls[PG_GET_WAL_STATS_COLS] = {0};
+ Assert(start_lsn <= end_lsn);
+
InitMaterializedSRF(fcinfo, 0);
xlogreader = InitXLogReaderState(start_lsn);
@@ -790,46 +737,54 @@ GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
}
/*
- * Get stats of all WAL records between start LSN and end LSN.
+ * pg_get_wal_stats
*
- * 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.
+ * Get stats of all WAL records between start LSN and end LSN.
*/
Datum
pg_get_wal_stats(PG_FUNCTION_ARGS)
{
- XLogRecPtr start_lsn;
- XLogRecPtr end_lsn;
- bool stats_per_record;
-
- start_lsn = PG_GETARG_LSN(0);
- end_lsn = PG_GETARG_LSN(1);
- stats_per_record = PG_GETARG_BOOL(2);
-
- end_lsn = ValidateInputLSNs(false, start_lsn, end_lsn);
+ XLogRecPtr start_lsn = PG_GETARG_LSN(0);
+ XLogRecPtr end_lsn = PG_GETARG_LSN(1);
+ bool stats_per_record = PG_GETARG_BOOL(2);
+ ValidateInputLSNs(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.
- *
- * This function emits an error if a future start i.e. WAL LSN the database
- * system doesn't know about is specified.
+ * The following functions have been removed in newer versions in 1.1, but
+ * they are kept around for compatibility.
*/
+Datum
+pg_get_wal_records_info_till_end_of_wal(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr start_lsn = PG_GETARG_LSN(0);
+ XLogRecPtr end_lsn = GetCurrentLSN();
+
+ if (start_lsn > end_lsn)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("WAL start LSN must be less than current LSN")));
+
+ GetWALRecordsInfo(fcinfo, start_lsn, end_lsn);
+
+ PG_RETURN_VOID();
+}
+
Datum
pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS)
{
- XLogRecPtr start_lsn;
- XLogRecPtr end_lsn = InvalidXLogRecPtr;
- bool stats_per_record;
+ XLogRecPtr start_lsn = PG_GETARG_LSN(0);
+ XLogRecPtr end_lsn = GetCurrentLSN();
+ bool stats_per_record = PG_GETARG_BOOL(1);
- start_lsn = PG_GETARG_LSN(0);
- stats_per_record = PG_GETARG_BOOL(1);
-
- end_lsn = ValidateInputLSNs(true, start_lsn, end_lsn);
+ if (start_lsn > end_lsn)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("WAL start LSN must be less than current LSN")));
GetWalStats(fcinfo, start_lsn, end_lsn, stats_per_record);
diff --git a/contrib/pg_walinspect/sql/oldextversions.sql b/contrib/pg_walinspect/sql/oldextversions.sql
index f448094add..39e926a096 100644
--- a/contrib/pg_walinspect/sql/oldextversions.sql
+++ b/contrib/pg_walinspect/sql/oldextversions.sql
@@ -2,13 +2,31 @@
CREATE EXTENSION pg_walinspect WITH VERSION '1.0';
+-- Mask DETAIL messages as these could refer to current LSN positions.
+\set VERBOSITY terse
+
-- List what version 1.0 contains
\dx+ pg_walinspect
+-- Make sure checkpoints don't interfere with the test.
+SELECT 'init' FROM pg_create_physical_replication_slot('regress_pg_walinspect_slot', true, false);
+
+CREATE TABLE sample_tbl(col1 int, col2 int);
+SELECT pg_current_wal_lsn() AS wal_lsn1 \gset
+INSERT INTO sample_tbl SELECT * FROM generate_series(1, 2);
+
+-- Check bounds for these past functions.
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info_till_end_of_wal(:'wal_lsn1');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
+
-- Move to new version 1.1
ALTER EXTENSION pg_walinspect UPDATE TO '1.1';
-- List what version 1.1 contains
\dx+ pg_walinspect
+SELECT pg_drop_replication_slot('regress_pg_walinspect_slot');
+
DROP EXTENSION pg_walinspect;
diff --git a/contrib/pg_walinspect/sql/pg_walinspect.sql b/contrib/pg_walinspect/sql/pg_walinspect.sql
index 766ed6b87d..ba4d17df6c 100644
--- a/contrib/pg_walinspect/sql/pg_walinspect.sql
+++ b/contrib/pg_walinspect/sql/pg_walinspect.sql
@@ -33,9 +33,7 @@ SELECT * FROM pg_get_wal_block_info(:'wal_lsn2', :'wal_lsn1');
-- LSNs with the highest value possible.
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
--- failures with end LSNs
+-- Success with end LSNs.
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
@@ -49,8 +47,6 @@ SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFF
-- ===================================================================
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_record_info(:'wal_lsn1');
-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_stats_till_end_of_wal(:'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_stats(:'wal_lsn1', :'wal_lsn2');
SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', :'wal_lsn2');
--
2.39.2
signature.asc
Description: PGP signature
