Refactor and improve tests of pg_walinspect

michaelpq · michaelpq · commit 1f282c24e464 · 2023-03-13T13:03:29.000+09:00
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
@@ -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
@@ -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
@@ -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,17 +9,52 @@ 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
 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
@@ -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
@@ -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
@@ -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;
