Refactor and improve tests of pg_walinspect

The regression tests of pg_walinspect are reworked on a few aspects:
- Reorganization on the validation checks done for the start and end
LSNs on the six SQL functions currently available in 1.1.
- Addition of a few patterns doing bound checks for invalid start LSN,
invalid end LSN, and failures in reading LSN positions, for anything
that's been missing.
- Use of a consistent style across the whole, limiting blank lines
across the queries.
- Addition of a new test script for upgrades.  For the time being, this
is straight-forward with a check that the upgrade from 1.0 works
correctly.  This will be made more complicated once the interface of
this extension is reworked in 1.1 with a follow-up patch.

Most of the contents of this commit are extracted from a larger patch by
the same author, largely reorganized by me to minimize the differences
with the upcoming work aimed to lift the bound checks on the input LSNs
used by the functions of this extension.

Author: Bharath Rupireddy, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACU0_q-o4DSweyaW9NO1KBx-QkN6G_OzYQvpjf3CZVASkg@mail.gmail.com

diff --git a/contrib/pg_walinspect/Makefile b/contrib/pg_walinspect/Makefile
index 7033878a79ee44c548b2a90add5048ea81e092b9..22090f7716c8cf6154be8537e0947259a14f4eab 100644 (file)
--- a/contrib/pg_walinspect/Makefile
+++ b/contrib/pg_walinspect/Makefile
@@ -9,7 +9,7 @@ PGFILEDESC = "pg_walinspect - functions to inspect contents of PostgreSQL Write-
 EXTENSION = pg_walinspect
 DATA = pg_walinspect--1.0.sql pg_walinspect--1.0--1.1.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 (file)
index 0000000..0db3538
--- /dev/null
+++ b/contrib/pg_walinspect/expected/oldextversions.out
@@ -0,0 +1,30 @@
+-- test old extension version entry points
+CREATE EXTENSION pg_walinspect WITH VERSION '1.0';
+-- List what version 1.0 contains
+\dx+ pg_walinspect
+           Objects in extension "pg_walinspect"
+                    Object description                     
+-----------------------------------------------------------
+ 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)
+(5 rows)
+
+-- 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                     
+-----------------------------------------------------------
+ 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)
+
+DROP EXTENSION pg_walinspect;

diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out
index e0eb7ca08f62e7ae54d968ab7f22277036323722..c9fb781055e284bf9c25d6ee4797fc7ea487fbca 100644 (file)
--- a/contrib/pg_walinspect/expected/pg_walinspect.out
+++ b/contrib/pg_walinspect/expected/pg_walinspect.out
@@ -1,4 +1,6 @@
 CREATE EXTENSION pg_walinspect;
+-- Mask DETAIL messages as these could refer to current LSN positions.
+\set VERBOSITY terse
 -- Make sure checkpoints don't interfere with the test.
 SELECT 'init' FROM pg_create_physical_replication_slot('regress_pg_walinspect_slot', true, false);
  ?column? 
@@ -7,6 +9,7 @@ SELECT 'init' FROM pg_create_physical_replication_slot('regress_pg_walinspect_sl
 (1 row)
 
 CREATE TABLE sample_tbl(col1 int, col2 int);
+-- Save some LSNs for comparisons
 SELECT pg_current_wal_lsn() AS wal_lsn1 \gset
 INSERT INTO sample_tbl SELECT * FROM generate_series(1, 2);
 SELECT pg_current_wal_lsn() AS wal_lsn2 \gset
@@ -14,10 +17,44 @@ 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(:'wal_lsn2', :'wal_lsn1'); -- ERROR
+-- Invalid input LSN.
+SELECT * FROM pg_get_wal_record_info('0/0');
+ERROR:  could not read WAL at LSN 0/0
+-- Invalid start LSN.
+SELECT * FROM pg_get_wal_records_info('0/0', :'wal_lsn1');
+ERROR:  could not read WAL at LSN 0/0
+SELECT * FROM pg_get_wal_stats('0/0', :'wal_lsn1');
+ERROR:  could not read WAL at LSN 0/0
+SELECT * FROM pg_get_wal_block_info('0/0', :'wal_lsn1');
+ERROR:  could not read WAL at LSN 0/0
+-- Start LSN > End LSN.
+SELECT * FROM pg_get_wal_records_info(:'wal_lsn2', :'wal_lsn1');
 ERROR:  WAL start LSN must be less than end LSN
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1'); -- ERROR
+SELECT * FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1');
 ERROR:  WAL start LSN must be less than end LSN
+SELECT * FROM pg_get_wal_block_info(:'wal_lsn2', :'wal_lsn1');
+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
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
+ERROR:  cannot accept future end LSN
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
+ERROR:  cannot accept future end LSN
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
+ERROR:  cannot accept future end LSN
+-- 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
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
+ERROR:  cannot accept future start LSN
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
+ERROR:  cannot accept future start LSN
 -- ===================================================================
 -- Tests for all function executions
 -- ===================================================================
@@ -27,13 +64,19 @@ 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(:'wal_lsn1', :'wal_lsn2');
+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_records_info_till_end_of_wal(:'wal_lsn1');
+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 
 ----
  t
@@ -45,7 +88,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_block_info(:'wal_lsn1', :'wal_lsn2');
  ok 
 ----
  t
@@ -220,3 +263,4 @@ SELECT pg_drop_replication_slot('regress_pg_walinspect_slot');
 (1 row)
 
 DROP TABLE sample_tbl;
+DROP EXTENSION pg_walinspect;

diff --git a/contrib/pg_walinspect/meson.build b/contrib/pg_walinspect/meson.build
index bf7b79b1b72c293708f43727c38ada954cb56244..80059f61197d514f32d56929d4d6e4347624eb4f 100644 (file)
--- a/contrib/pg_walinspect/meson.build
+++ b/contrib/pg_walinspect/meson.build
@@ -30,6 +30,7 @@ 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).

diff --git a/contrib/pg_walinspect/sql/oldextversions.sql b/contrib/pg_walinspect/sql/oldextversions.sql
new file mode 100644 (file)
index 0000000..f448094
--- /dev/null
+++ b/contrib/pg_walinspect/sql/oldextversions.sql
@@ -0,0 +1,14 @@
+-- test old extension version entry points
+
+CREATE EXTENSION pg_walinspect WITH VERSION '1.0';
+
+-- List what version 1.0 contains
+\dx+ pg_walinspect
+
+-- Move to new version 1.1
+ALTER EXTENSION pg_walinspect UPDATE TO '1.1';
+
+-- List what version 1.1 contains
+\dx+ pg_walinspect
+
+DROP EXTENSION pg_walinspect;

diff --git a/contrib/pg_walinspect/sql/pg_walinspect.sql b/contrib/pg_walinspect/sql/pg_walinspect.sql
index 01a120f398073552883d51b1573802be1a392a67..766ed6b87d1b2bf6a6f438f9a881a7a6472e0185 100644 (file)
--- a/contrib/pg_walinspect/sql/pg_walinspect.sql
+++ b/contrib/pg_walinspect/sql/pg_walinspect.sql
@@ -1,39 +1,59 @@
 CREATE EXTENSION pg_walinspect;
 
+-- Mask DETAIL messages as these could refer to current LSN positions.
+\set VERBOSITY terse
+
 -- 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);
 
+-- Save some LSNs for comparisons
 SELECT pg_current_wal_lsn() AS wal_lsn1 \gset
-
 INSERT INTO sample_tbl SELECT * FROM generate_series(1, 2);
-
 SELECT pg_current_wal_lsn() AS wal_lsn2 \gset
-
 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(:'wal_lsn2', :'wal_lsn1'); -- ERROR
-
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1'); -- ERROR
+-- Invalid input LSN.
+SELECT * FROM pg_get_wal_record_info('0/0');
+
+-- Invalid start LSN.
+SELECT * FROM pg_get_wal_records_info('0/0', :'wal_lsn1');
+SELECT * FROM pg_get_wal_stats('0/0', :'wal_lsn1');
+SELECT * FROM pg_get_wal_block_info('0/0', :'wal_lsn1');
+
+-- Start LSN > End LSN.
+SELECT * FROM pg_get_wal_records_info(:'wal_lsn2', :'wal_lsn1');
+SELECT * FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1');
+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
+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');
+-- failures with start LSNs
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
+SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
 
 -- ===================================================================
 -- Tests for all function executions
 -- ===================================================================
 
 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_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_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');
 
 -- ===================================================================
 -- Test for filtering out WAL records of a particular table
@@ -80,29 +100,22 @@ CREATE ROLE regress_pg_walinspect;
 
 SELECT has_function_privilege('regress_pg_walinspect',
   'pg_get_wal_record_info(pg_lsn)', 'EXECUTE'); -- no
-
 SELECT has_function_privilege('regress_pg_walinspect',
   'pg_get_wal_records_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- no
-
 SELECT has_function_privilege('regress_pg_walinspect',
   'pg_get_wal_stats(pg_lsn, pg_lsn, boolean) ', 'EXECUTE'); -- no
-
 SELECT has_function_privilege('regress_pg_walinspect',
   'pg_get_wal_block_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- no
 
 -- Functions accessible by users with role pg_read_server_files
 
 GRANT pg_read_server_files TO regress_pg_walinspect;
-
 SELECT has_function_privilege('regress_pg_walinspect',
   'pg_get_wal_record_info(pg_lsn)', 'EXECUTE'); -- yes
-
 SELECT has_function_privilege('regress_pg_walinspect',
   'pg_get_wal_records_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes
-
 SELECT has_function_privilege('regress_pg_walinspect',
   'pg_get_wal_stats(pg_lsn, pg_lsn, boolean) ', 'EXECUTE'); -- yes
-
 SELECT has_function_privilege('regress_pg_walinspect',
   'pg_get_wal_block_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes
 
@@ -111,37 +124,28 @@ REVOKE pg_read_server_files FROM regress_pg_walinspect;
 -- Superuser can grant execute to other users
 GRANT EXECUTE ON FUNCTION pg_get_wal_record_info(pg_lsn)
   TO regress_pg_walinspect;
-
 GRANT EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn)
   TO regress_pg_walinspect;
-
 GRANT EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean)
   TO regress_pg_walinspect;
-
 GRANT EXECUTE ON FUNCTION pg_get_wal_block_info(pg_lsn, pg_lsn)
   TO regress_pg_walinspect;
 
 SELECT has_function_privilege('regress_pg_walinspect',
   'pg_get_wal_record_info(pg_lsn)', 'EXECUTE'); -- yes
-
 SELECT has_function_privilege('regress_pg_walinspect',
   'pg_get_wal_records_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes
-
 SELECT has_function_privilege('regress_pg_walinspect',
   'pg_get_wal_stats(pg_lsn, pg_lsn, boolean) ', 'EXECUTE'); -- yes
-
 SELECT has_function_privilege('regress_pg_walinspect',
   'pg_get_wal_block_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes
 
 REVOKE EXECUTE ON FUNCTION pg_get_wal_record_info(pg_lsn)
   FROM regress_pg_walinspect;
-
 REVOKE EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn)
   FROM regress_pg_walinspect;
-
 REVOKE EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean)
   FROM regress_pg_walinspect;
-
 REVOKE EXECUTE ON FUNCTION pg_get_wal_block_info(pg_lsn, pg_lsn)
   FROM regress_pg_walinspect;
 
@@ -154,3 +158,4 @@ DROP ROLE regress_pg_walinspect;
 SELECT pg_drop_replication_slot('regress_pg_walinspect_slot');
 
 DROP TABLE sample_tbl;
+DROP EXTENSION pg_walinspect;